From dbfcce34e73e71b9ea5a7c417c23377ac6ef24d1 Mon Sep 17 00:00:00 2001 From: Scott Sadler Date: Thu, 23 Feb 2017 12:07:38 +0100 Subject: [PATCH 01/23] voting.py --- bigchaindb/voting.py | 47 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 bigchaindb/voting.py diff --git a/bigchaindb/voting.py b/bigchaindb/voting.py new file mode 100644 index 00000000..0138e783 --- /dev/null +++ b/bigchaindb/voting.py @@ -0,0 +1,47 @@ +import collections + + +def filter_eligible_votes(votes, block_voters, keyring, check_signature): + """ + Filter votes from unknown nodes or nodes that are not listed on + block. Here is our sybill protection. + """ + eligible_voters = set(keyring) & set(block_voters) + eligible_votes = [] + + for vote in votes: + pubkey = vote['node_pubkey'] + voter_eligible = pubkey in eligible_voters + sig_legit = sig_is_legit(vote) + if voter_eligible and sig_legit: + eligible_votes[pubkey].append(vote) + + return eligible_votes + + +def count_votes(eligible_votes, check_schema): + by_voter = collections.defaultdict(list) + for vote in eligible_votes: + by_voter[vote['node_pubkey']].append(vote) + + n_valid = 0 + n_invalid = 0 + prev_blocks = collections.Counter() + + for pubkey, votes in by_voter.items(): + if len(votes) > 1 or not schema_is_correct(votes[0]): + n_invalid += 1 + continue + + vote = votes[0] + prev_blocks[vote['vote']['previous_block']] += 1 + if vote['vote']['is_block_valid']: + n_valid += 1 + else: + n_invalid += 1 + + return { + 'valid': n_valid, + 'invalid': n_invalid, + 'prev_block': prev_blocks.most_common()[0] + } From d71e560ba4b29c868cec497183054c1d8fede861 Mon Sep 17 00:00:00 2001 From: Scott Sadler Date: Thu, 23 Feb 2017 15:08:06 +0100 Subject: [PATCH 02/23] flesh out voting module --- bigchaindb/voting.py | 84 +++++++++++++++++++++++++++++++++++--------- tests/test_voting.py | 61 ++++++++++++++++++++++++++++++++ 2 files changed, 129 insertions(+), 16 deletions(-) create mode 100644 tests/test_voting.py diff --git a/bigchaindb/voting.py b/bigchaindb/voting.py index 0138e783..159f631f 100644 --- a/bigchaindb/voting.py +++ b/bigchaindb/voting.py @@ -1,25 +1,41 @@ +""" +Everything to do with creating and checking votes. +All functions in this module should be referentially transparent, that is, +they always give the same output for a given input. This makes it easier +to test. +""" import collections -def filter_eligible_votes(votes, block_voters, keyring, check_signature): +def partition_eligible_votes(votes, eligible_voters, verify_vote_signature): """ Filter votes from unknown nodes or nodes that are not listed on - block. Here is our sybill protection. + block. This is the primary Sybill protection. """ - eligible_voters = set(keyring) & set(block_voters) - eligible_votes = [] + eligible, ineligible = ([], []) for vote in votes: - pubkey = vote['node_pubkey'] - voter_eligible = pubkey in eligible_voters - sig_legit = sig_is_legit(vote) - if voter_eligible and sig_legit: - eligible_votes[pubkey].append(vote) + voter_eligible = vote['node_pubkey'] in eligible_voters + if voter_eligible and verify_vote_signature(vote): + eligible.append(vote) + else: + ineligible.append(vote) - return eligible_votes + return eligible, ineligible -def count_votes(eligible_votes, check_schema): +def count_votes(eligible_votes): + """ + Given a list of eligible votes, (votes from known nodes that are listed + as voters), count the votes to produce three quantities: + + Number of votes that say valid + Number of votes that say invalid + Highest agreement on previous block ID + + Also, detect if there are multiple votes from a single node and return them + in a separate "cheat" dictionary. + """ by_voter = collections.defaultdict(list) for vote in eligible_votes: by_voter[vote['node_pubkey']].append(vote) @@ -27,9 +43,11 @@ def count_votes(eligible_votes, check_schema): n_valid = 0 n_invalid = 0 prev_blocks = collections.Counter() + cheat = {} for pubkey, votes in by_voter.items(): - if len(votes) > 1 or not schema_is_correct(votes[0]): + if len(votes) > 1: + cheat[pubkey] = votes n_invalid += 1 continue @@ -41,7 +59,41 @@ def count_votes(eligible_votes, check_schema): n_invalid += 1 return { - 'valid': n_valid, - 'invalid': n_invalid, - 'prev_block': prev_blocks.most_common()[0] - } + 'n_valid': n_valid, + 'n_invalid': n_invalid, + 'n_agree_prev_block': prev_blocks.most_common()[0][1] + }, cheat + + +def decide_votes(n_voters, n_valid, n_invalid, n_agree_prev_block): + """ + Decide on votes. + + To return VALID there must be a clear majority that say VALID + and also agree on the previous block. This is achieved using the > operator. + + A tie on an even number of votes counts as INVALID so the >= operator is + used. + """ + + # Check insane cases. This is basic, not exhaustive. + if n_valid + n_invalid > n_voters or n_agree_prev_block > n_voters: + raise ValueError('Arguments not sane: %s' % { + 'n_voters': n_voters, + 'n_valid': n_valid, + 'n_invalid': n_invalid, + 'n_agree_prev_block': n_agree_prev_block, + }) + + if n_invalid * 2 >= n_voters: + return INVALID + if n_valid * 2 > n_voters: + if n_agree_prev_block * 2 > n_voters: + return VALID + return INVALID + return UNDECIDED + + +INVALID = 'invalid' +VALID = TX_VALID = 'valid' +UNDECIDED = TX_UNDECIDED = 'undecided' diff --git a/tests/test_voting.py b/tests/test_voting.py new file mode 100644 index 00000000..67c5c284 --- /dev/null +++ b/tests/test_voting.py @@ -0,0 +1,61 @@ +import pytest + +from bigchaindb.core import Bigchain +from bigchaindb.voting import (count_votes, partition_eligible_votes, + decide_votes, INVALID, VALID, UNDECIDED) + + +def test_partition_eligible_votes(): + nodes = list(map(Bigchain, 'abc')) + votes = [n.vote('block', 'a', True) for n in nodes] + + el, inel = partition_eligible_votes(votes, 'abc', lambda _: True) + + assert el == votes + assert inel == [] + + +def test_count_votes(): + nodes = list(map(Bigchain, 'abc')) + votes = [n.vote('block', 'a', True) for n in nodes] + + assert count_votes(votes) == ({ + 'n_valid': 3, + 'n_invalid': 0, + 'n_agree_prev_block': 3 + }, {}) + + +DECISION_TESTS = [dict( + zip(['n_voters', 'n_valid', 'n_invalid', 'n_agree_prev_block'], t)) + for t in [ + (1, 1, 1, 1), + (2, 2, 1, 2), + (3, 2, 2, 2), + (4, 3, 2, 3), + (5, 3, 3, 3), + (6, 4, 3, 4), + (7, 4, 4, 4), + (8, 5, 4, 5), + ] +] + + +@pytest.mark.parametrize('kwargs', DECISION_TESTS) +def test_decide_votes_valid(kwargs): + kwargs = kwargs.copy() + kwargs['n_invalid'] = 0 + assert decide_votes(**kwargs) == VALID + kwargs['n_agree_prev_block'] -= 1 + assert decide_votes(**kwargs) == INVALID + kwargs['n_valid'] -= 1 + assert decide_votes(**kwargs) == UNDECIDED + + +@pytest.mark.parametrize('kwargs', DECISION_TESTS) +def test_decide_votes_invalid(kwargs): + kwargs = kwargs.copy() + kwargs['n_valid'] = 0 + assert decide_votes(**kwargs) == INVALID + kwargs['n_invalid'] -= 1 + assert decide_votes(**kwargs) == UNDECIDED From 20f6539e10a7fa674b7a4c1f72597b1ca59bf82c Mon Sep 17 00:00:00 2001 From: Scott Sadler Date: Thu, 23 Feb 2017 17:29:46 +0100 Subject: [PATCH 03/23] check count_votes invalid input --- bigchaindb/voting.py | 10 +++++----- tests/test_voting.py | 17 +++++++++++++++++ 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/bigchaindb/voting.py b/bigchaindb/voting.py index 159f631f..af12f691 100644 --- a/bigchaindb/voting.py +++ b/bigchaindb/voting.py @@ -7,6 +7,11 @@ to test. import collections +VALID = 'valid' +INVALID = 'invalid' +UNDECIDED = 'undecided' + + def partition_eligible_votes(votes, eligible_voters, verify_vote_signature): """ Filter votes from unknown nodes or nodes that are not listed on @@ -92,8 +97,3 @@ def decide_votes(n_voters, n_valid, n_invalid, n_agree_prev_block): return VALID return INVALID return UNDECIDED - - -INVALID = 'invalid' -VALID = TX_VALID = 'valid' -UNDECIDED = TX_UNDECIDED = 'undecided' diff --git a/tests/test_voting.py b/tests/test_voting.py index 67c5c284..33be2fde 100644 --- a/tests/test_voting.py +++ b/tests/test_voting.py @@ -5,6 +5,10 @@ from bigchaindb.voting import (count_votes, partition_eligible_votes, decide_votes, INVALID, VALID, UNDECIDED) +################################################################################ +# Tests for checking vote eligibility + + def test_partition_eligible_votes(): nodes = list(map(Bigchain, 'abc')) votes = [n.vote('block', 'a', True) for n in nodes] @@ -26,6 +30,10 @@ def test_count_votes(): }, {}) +################################################################################ +# Tests for vote decision making + + DECISION_TESTS = [dict( zip(['n_voters', 'n_valid', 'n_invalid', 'n_agree_prev_block'], t)) for t in [ @@ -59,3 +67,12 @@ def test_decide_votes_invalid(kwargs): assert decide_votes(**kwargs) == INVALID kwargs['n_invalid'] -= 1 assert decide_votes(**kwargs) == UNDECIDED + + +def test_decide_votes_checks_arguments(): + with pytest.raises(ValueError): + decide_votes(n_voters=1, n_valid=2, n_invalid=0, n_agree_prev_block=0) + with pytest.raises(ValueError): + decide_votes(n_voters=1, n_valid=0, n_invalid=2, n_agree_prev_block=0) + with pytest.raises(ValueError): + decide_votes(n_voters=1, n_valid=0, n_invalid=0, n_agree_prev_block=2) From fdad8cd79687fc78f45fead2b748a4cb6cb113c5 Mon Sep 17 00:00:00 2001 From: Scott Sadler Date: Thu, 23 Feb 2017 17:58:08 +0100 Subject: [PATCH 04/23] Static Classify Voting --- bigchaindb/voting.py | 238 +++++++++++++++++++++++++++---------------- tests/test_voting.py | 36 ++++--- 2 files changed, 174 insertions(+), 100 deletions(-) diff --git a/bigchaindb/voting.py b/bigchaindb/voting.py index af12f691..dcef47b6 100644 --- a/bigchaindb/voting.py +++ b/bigchaindb/voting.py @@ -1,10 +1,7 @@ -""" -Everything to do with creating and checking votes. -All functions in this module should be referentially transparent, that is, -they always give the same output for a given input. This makes it easier -to test. -""" import collections +from bigchaindb.common.schema import SchemaValidationError, validate_vote_schema +from bigchaindb.common.utils import serialize +from bigchaindb.common.crypto import PublicKey VALID = 'valid' @@ -12,88 +9,159 @@ INVALID = 'invalid' UNDECIDED = 'undecided' -def partition_eligible_votes(votes, eligible_voters, verify_vote_signature): +class Voting: """ - Filter votes from unknown nodes or nodes that are not listed on - block. This is the primary Sybill protection. - """ - eligible, ineligible = ([], []) + Everything to do with creating and checking votes. - for vote in votes: - voter_eligible = vote['node_pubkey'] in eligible_voters - if voter_eligible and verify_vote_signature(vote): - eligible.append(vote) - else: - ineligible.append(vote) + All functions in this class should be referentially transparent, that is, + they always give the same output for a given input. This makes it easier + to test. This also means no logging! - return eligible, ineligible - - -def count_votes(eligible_votes): - """ - Given a list of eligible votes, (votes from known nodes that are listed - as voters), count the votes to produce three quantities: - - Number of votes that say valid - Number of votes that say invalid - Highest agreement on previous block ID - - Also, detect if there are multiple votes from a single node and return them - in a separate "cheat" dictionary. - """ - by_voter = collections.defaultdict(list) - for vote in eligible_votes: - by_voter[vote['node_pubkey']].append(vote) - - n_valid = 0 - n_invalid = 0 - prev_blocks = collections.Counter() - cheat = {} - - for pubkey, votes in by_voter.items(): - if len(votes) > 1: - cheat[pubkey] = votes - n_invalid += 1 - continue - - vote = votes[0] - prev_blocks[vote['vote']['previous_block']] += 1 - if vote['vote']['is_block_valid']: - n_valid += 1 - else: - n_invalid += 1 - - return { - 'n_valid': n_valid, - 'n_invalid': n_invalid, - 'n_agree_prev_block': prev_blocks.most_common()[0][1] - }, cheat - - -def decide_votes(n_voters, n_valid, n_invalid, n_agree_prev_block): - """ - Decide on votes. - - To return VALID there must be a clear majority that say VALID - and also agree on the previous block. This is achieved using the > operator. - - A tie on an even number of votes counts as INVALID so the >= operator is - used. + Assumptions regarding data: + * Vote is a dictionary, but it is not assumed that any properties are. + * Everything else is assumed to be structurally correct, otherwise errors + may be thrown. """ - # Check insane cases. This is basic, not exhaustive. - if n_valid + n_invalid > n_voters or n_agree_prev_block > n_voters: - raise ValueError('Arguments not sane: %s' % { - 'n_voters': n_voters, - 'n_valid': n_valid, - 'n_invalid': n_invalid, - 'n_agree_prev_block': n_agree_prev_block, - }) + @classmethod + def block_election(cls, block, votes, keyring): + """ + Calculate the election status of a block. + """ + eligible_voters = set(block['voters']) & set(keyring) + eligible_votes, ineligible_votes = \ + cls.partition_eligible_votes(votes, eligible_voters) + results = cls.count_votes(eligible_votes) + results['status'] = decide_votes(results['counts']) + results['ineligible'] = ineligible_votes + return results - if n_invalid * 2 >= n_voters: - return INVALID - if n_valid * 2 > n_voters: - if n_agree_prev_block * 2 > n_voters: - return VALID - return INVALID - return UNDECIDED + @classmethod + def partition_eligible_votes(cls, votes, eligible_voters): + """ + Filter votes from unknown nodes or nodes that are not listed on + block. This is the primary Sybill protection. + """ + eligible, ineligible = ([], []) + + for vote in votes: + voter_eligible = vote.get('node_pubkey') in eligible_voters + if voter_eligible and cls.verify_vote_signature(vote): + eligible.append(vote) + else: + ineligible.append(vote) + + return eligible, ineligible + + @classmethod + def count_votes(cls, eligible_votes): + """ + Given a list of eligible votes, (votes from known nodes that are listed + as voters), count the votes to produce three quantities: + + Number of votes that say valid + Number of votes that say invalid + Highest agreement on previous block ID + + Also, detect if there are multiple votes from a single node and return them + in a separate "cheat" dictionary. + """ + by_voter = collections.defaultdict(list) + for vote in eligible_votes: + by_voter[vote['node_pubkey']].append(vote) + + n_valid = 0 + n_invalid = 0 + prev_blocks = collections.Counter() + cheat = [] + malformed = [] + + for pubkey, votes in by_voter.items(): + if len(votes) > 1: + cheat.append(votes) + n_invalid += 1 + continue + + vote = votes[0] + + if not cls.verify_vote_schema(vote): + malformed.append(vote) + n_invalid += 1 + continue + + prev_blocks[vote['vote']['previous_block']] += 1 + if vote['vote']['is_block_valid']: + n_valid += 1 + else: + n_invalid += 1 + + return { + 'counts': { + 'n_valid': n_valid, + 'n_invalid': n_invalid, + 'n_agree_prev_block': prev_blocks.most_common()[0][1], + }, + 'cheat': cheat, + 'malformed': malformed, + } + + @classmethod + def decide_votes(cls, n_voters, n_valid, n_invalid, n_agree_prev_block): + """ + Decide on votes. + + To return VALID there must be a clear majority that say VALID + and also agree on the previous block. This is achieved using the > operator. + + A tie on an even number of votes counts as INVALID so the >= operator is + used. + """ + + # Check insane cases. This is basic, not exhaustive. + if n_valid + n_invalid > n_voters or n_agree_prev_block > n_voters: + raise ValueError('Arguments not sane: %s' % { + 'n_voters': n_voters, + 'n_valid': n_valid, + 'n_invalid': n_invalid, + 'n_agree_prev_block': n_agree_prev_block, + }) + + if n_invalid * 2 >= n_voters: + return INVALID + if n_valid * 2 > n_voters: + if n_agree_prev_block * 2 > n_voters: + return VALID + return INVALID + return UNDECIDED + + @classmethod + def verify_vote_signature(cls, vote): + """Verify the signature of a vote + + A valid vote should have been signed by a voter's private key. + + Args: + vote (list): voters of the block that is under election + + Returns: + bool: True if the signature is correct, False otherwise. + """ + signature = vote.get('signature') + pk_base58 = vote.get('node_pubkey') + + if not (type(signature) == str and type(pk_base58) == str): + raise ValueError("Malformed vote: %s" % vote) + + public_key = PublicKey(pk_base58) + body = serialize(signed_vote['vote']).encode() + return public_key.verify(body, signature) + + @classmethod + def verify_vote_schema(cls, vote): + # I'm not sure this is the correct approach. Maybe we should allow + # duck typing w/r/t votes. + try: + validate_vote_schema(vote) + return True + except SchemaValidationError: + return False diff --git a/tests/test_voting.py b/tests/test_voting.py index 33be2fde..2d7b723f 100644 --- a/tests/test_voting.py +++ b/tests/test_voting.py @@ -1,33 +1,36 @@ import pytest +from unittest.mock import patch from bigchaindb.core import Bigchain -from bigchaindb.voting import (count_votes, partition_eligible_votes, - decide_votes, INVALID, VALID, UNDECIDED) +from bigchaindb.voting import Voting, INVALID, VALID, UNDECIDED ################################################################################ # Tests for checking vote eligibility -def test_partition_eligible_votes(): +@patch('bigchaindb.voting.Voting.verify_vote_signature') +def test_partition_eligible_votes(_): nodes = list(map(Bigchain, 'abc')) votes = [n.vote('block', 'a', True) for n in nodes] - el, inel = partition_eligible_votes(votes, 'abc', lambda _: True) + el, inel = Voting.partition_eligible_votes(votes, 'abc') assert el == votes assert inel == [] -def test_count_votes(): +@patch('bigchaindb.voting.Voting.verify_vote_schema') +def test_count_votes(_): nodes = list(map(Bigchain, 'abc')) + votes = [n.vote('block', 'a', True) for n in nodes] - assert count_votes(votes) == ({ + assert Voting.count_votes(votes)['counts'] == { 'n_valid': 3, 'n_invalid': 0, 'n_agree_prev_block': 3 - }, {}) + } ################################################################################ @@ -53,26 +56,29 @@ DECISION_TESTS = [dict( def test_decide_votes_valid(kwargs): kwargs = kwargs.copy() kwargs['n_invalid'] = 0 - assert decide_votes(**kwargs) == VALID + assert Voting.decide_votes(**kwargs) == VALID kwargs['n_agree_prev_block'] -= 1 - assert decide_votes(**kwargs) == INVALID + assert Voting.decide_votes(**kwargs) == INVALID kwargs['n_valid'] -= 1 - assert decide_votes(**kwargs) == UNDECIDED + assert Voting.decide_votes(**kwargs) == UNDECIDED @pytest.mark.parametrize('kwargs', DECISION_TESTS) def test_decide_votes_invalid(kwargs): kwargs = kwargs.copy() kwargs['n_valid'] = 0 - assert decide_votes(**kwargs) == INVALID + assert Voting.decide_votes(**kwargs) == INVALID kwargs['n_invalid'] -= 1 - assert decide_votes(**kwargs) == UNDECIDED + assert Voting.decide_votes(**kwargs) == UNDECIDED def test_decide_votes_checks_arguments(): with pytest.raises(ValueError): - decide_votes(n_voters=1, n_valid=2, n_invalid=0, n_agree_prev_block=0) + Voting.decide_votes(n_voters=1, n_valid=2, n_invalid=0, + n_agree_prev_block=0) with pytest.raises(ValueError): - decide_votes(n_voters=1, n_valid=0, n_invalid=2, n_agree_prev_block=0) + Voting.decide_votes(n_voters=1, n_valid=0, n_invalid=2, + n_agree_prev_block=0) with pytest.raises(ValueError): - decide_votes(n_voters=1, n_valid=0, n_invalid=0, n_agree_prev_block=2) + Voting.decide_votes(n_voters=1, n_valid=0, n_invalid=0, + n_agree_prev_block=2) From c68856bc431d5d4d9989c657052f5675aac089d0 Mon Sep 17 00:00:00 2001 From: Scott Sadler Date: Thu, 23 Feb 2017 18:23:19 +0100 Subject: [PATCH 05/23] voting schema validate --- bigchaindb/voting.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/bigchaindb/voting.py b/bigchaindb/voting.py index dcef47b6..62eb27ee 100644 --- a/bigchaindb/voting.py +++ b/bigchaindb/voting.py @@ -32,7 +32,7 @@ class Voting: eligible_votes, ineligible_votes = \ cls.partition_eligible_votes(votes, eligible_voters) results = cls.count_votes(eligible_votes) - results['status'] = decide_votes(results['counts']) + results['status'] = cls.decide_votes(results['counts']) results['ineligible'] = ineligible_votes return results @@ -46,10 +46,15 @@ class Voting: for vote in votes: voter_eligible = vote.get('node_pubkey') in eligible_voters - if voter_eligible and cls.verify_vote_signature(vote): - eligible.append(vote) - else: - ineligible.append(vote) + if voter_eligible: + try: + cls.verify_vote_signature(vote) + except ValueError: + pass + else: + eligible.append(vote) + continue + ineligible.append(vote) return eligible, ineligible @@ -150,10 +155,10 @@ class Voting: pk_base58 = vote.get('node_pubkey') if not (type(signature) == str and type(pk_base58) == str): - raise ValueError("Malformed vote: %s" % vote) + raise ValueError('Malformed vote: %s' % vote) public_key = PublicKey(pk_base58) - body = serialize(signed_vote['vote']).encode() + body = serialize(vote['vote']).encode() return public_key.verify(body, signature) @classmethod From 7fd1de696c539da2057861a3a764486425aaebe4 Mon Sep 17 00:00:00 2001 From: Scott Sadler Date: Thu, 23 Feb 2017 18:31:40 +0100 Subject: [PATCH 06/23] move voting logic out of block_election_status --- bigchaindb/consensus.py | 32 ++------------- bigchaindb/core.py | 89 ++++++----------------------------------- bigchaindb/utils.py | 27 ------------- 3 files changed, 15 insertions(+), 133 deletions(-) diff --git a/bigchaindb/consensus.py b/bigchaindb/consensus.py index 0e7dc4bd..a0672577 100644 --- a/bigchaindb/consensus.py +++ b/bigchaindb/consensus.py @@ -1,11 +1,4 @@ -import logging - -from bigchaindb.utils import verify_vote_signature -from bigchaindb.common.schema import (SchemaValidationError, - validate_vote_schema) - - -logger = logging.getLogger(__name__) +from bigchaindb.voting import Voting class BaseConsensusRules(): @@ -16,34 +9,15 @@ class BaseConsensusRules(): All methods listed below must be implemented. """ + voting = Voting @staticmethod def validate_transaction(bigchain, transaction): """See :meth:`bigchaindb.models.Transaction.validate` - for documentation. - - """ + for documentation.""" return transaction.validate(bigchain) @staticmethod def validate_block(bigchain, block): """See :meth:`bigchaindb.models.Block.validate` for documentation.""" return block.validate(bigchain) - - @staticmethod - def verify_vote(voters, signed_vote): - """Verify the signature of a vote. - - Refer to the documentation of - :func:`bigchaindb.utils.verify_signature`. - """ - if verify_vote_signature(voters, signed_vote): - try: - validate_vote_schema(signed_vote) - return True - except SchemaValidationError as exc: - logger.warning(exc) - else: - logger.warning('Vote failed signature verification: ' - '%s with voters: %s', signed_vote, voters) - return False diff --git a/bigchaindb/core.py b/bigchaindb/core.py index b082eac4..d498a6d4 100644 --- a/bigchaindb/core.py +++ b/bigchaindb/core.py @@ -1,9 +1,6 @@ import random -import math -import collections from time import time -from itertools import compress from bigchaindb.common import crypto, exceptions from bigchaindb.common.utils import gen_timestamp, serialize from bigchaindb.common.transaction import TransactionLink @@ -203,8 +200,7 @@ class Bigchain(object): if include_status: if block: - status = self.block_election_status(block_id, - block['block']['voters']) + status = self.block_election_status(block) return block, status else: return block @@ -305,12 +301,8 @@ class Bigchain(object): blocks = backend.query.get_blocks_status_from_transaction(self.connection, txid) if blocks: # Determine the election status of each block - validity = { - block['id']: self.block_election_status( - block['id'], - block['block']['voters'] - ) for block in blocks - } + validity = {block['id']: self.block_election_status(block) + for block in blocks} # NOTE: If there are multiple valid blocks with this transaction, # something has gone wrong @@ -626,69 +618,12 @@ class Bigchain(object): # XXX: should this return instaces of Block? return backend.query.get_unvoted_blocks(self.connection, self.me) - def block_election_status(self, block_id, voters): - """Tally the votes on a block, and return the status: valid, invalid, or undecided.""" - - votes = list(backend.query.get_votes_by_block_id(self.connection, block_id)) - n_voters = len(voters) - - voter_counts = collections.Counter([vote['node_pubkey'] for vote in votes]) - for node in voter_counts: - if voter_counts[node] > 1: - raise exceptions.MultipleVotesError( - 'Block {block_id} has multiple votes ({n_votes}) from voting node {node_id}' - .format(block_id=block_id, n_votes=str(voter_counts[node]), node_id=node)) - - if len(votes) > n_voters: - raise exceptions.MultipleVotesError('Block {block_id} has {n_votes} votes cast, but only {n_voters} voters' - .format(block_id=block_id, n_votes=str(len(votes)), - n_voters=str(n_voters))) - - # vote_cast is the list of votes e.g. [True, True, False] - vote_cast = [vote['vote']['is_block_valid'] for vote in votes] - # prev_block are the ids of the nominal prev blocks e.g. - # ['block1_id', 'block1_id', 'block2_id'] - prev_block = [vote['vote']['previous_block'] for vote in votes] - # vote_validity checks whether a vote is valid - # or invalid, e.g. [False, True, True] - vote_validity = [self.consensus.verify_vote(voters, vote) for vote in votes] - - # element-wise product of stated vote and validity of vote - # vote_cast = [True, True, False] and - # vote_validity = [False, True, True] gives - # [True, False] - # Only the correctly signed votes are tallied. - vote_list = list(compress(vote_cast, vote_validity)) - - # Total the votes. Here, valid and invalid refer - # to the vote cast, not whether the vote itself - # is valid or invalid. - n_valid_votes = sum(vote_list) - n_invalid_votes = len(vote_cast) - n_valid_votes - - # The use of ceiling and floor is to account for the case of an - # even number of voters where half the voters have voted 'invalid' - # and half 'valid'. In this case, the block should be marked invalid - # to avoid a tie. In the case of an odd number of voters this is not - # relevant, since one side must be a majority. - if n_invalid_votes >= math.ceil(n_voters / 2): - return Bigchain.BLOCK_INVALID - elif n_valid_votes > math.floor(n_voters / 2): - # The block could be valid, but we still need to check if votes - # agree on the previous block. - # - # First, only consider blocks with legitimate votes - prev_block_list = list(compress(prev_block, vote_validity)) - # Next, only consider the blocks with 'yes' votes - prev_block_valid_list = list(compress(prev_block_list, vote_list)) - counts = collections.Counter(prev_block_valid_list) - # Make sure the majority vote agrees on previous node. - # The majority vote must be the most common, by definition. - # If it's not, there is no majority agreement on the previous - # block. - if counts.most_common()[0][1] > math.floor(n_voters / 2): - return Bigchain.BLOCK_VALID - else: - return Bigchain.BLOCK_INVALID - else: - return Bigchain.BLOCK_UNDECIDED + def block_election_status(self, block): + """Tally the votes on a block, and return the status: + valid, invalid, or undecided.""" + votes = list(backend.query.get_votes_by_block_id(self.connection, + block.id)) + keyring = self.nodes_except_me + [self.me] + result = self.consensus.voting.block_election(block, votes, keyring) + # TODO: logging + return result['status'] diff --git a/bigchaindb/utils.py b/bigchaindb/utils.py index 1860dd3e..4d7177d9 100644 --- a/bigchaindb/utils.py +++ b/bigchaindb/utils.py @@ -3,9 +3,6 @@ import threading import queue import multiprocessing as mp -from bigchaindb.common import crypto -from bigchaindb.common.utils import serialize - class ProcessGroup(object): @@ -116,30 +113,6 @@ def condition_details_has_owner(condition_details, owner): return False -def verify_vote_signature(voters, signed_vote): - """Verify the signature of a vote - - A valid vote should have been signed by a voter's private key. - - Args: - voters (list): voters of the block that is under election - signed_vote (dict): a vote with the `signature` included. - - Returns: - bool: True if the signature is correct, False otherwise. - """ - - signature = signed_vote['signature'] - pk_base58 = signed_vote['node_pubkey'] - - # immediately return False if the voter is not in the block voter list - if pk_base58 not in voters: - return False - - public_key = crypto.PublicKey(pk_base58) - return public_key.verify(serialize(signed_vote['vote']).encode(), signature) - - def is_genesis_block(block): """Check if the block is the genesis block. From ff7e4d11d1d05ec247f569e07202d00bc99761a1 Mon Sep 17 00:00:00 2001 From: Scott Sadler Date: Thu, 23 Feb 2017 18:56:39 +0100 Subject: [PATCH 07/23] remove test from test_core.py which is just wrong --- tests/test_core.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/tests/test_core.py b/tests/test_core.py index 6bcabdc9..d21e630d 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -80,16 +80,6 @@ def test_get_blocks_status_containing_tx(monkeypatch): bigchain.get_blocks_status_containing_tx('txid') -def test_has_previous_vote(monkeypatch): - from bigchaindb.core import Bigchain - monkeypatch.setattr( - 'bigchaindb.utils.verify_vote_signature', lambda voters, vote: False) - bigchain = Bigchain(public_key='pubkey', private_key='privkey') - block = {'votes': ({'node_pubkey': 'pubkey'},)} - with pytest.raises(Exception): - bigchain.has_previous_vote(block) - - @pytest.mark.parametrize('exists', (True, False)) def test_transaction_exists(monkeypatch, exists): from bigchaindb.core import Bigchain From 89e76ffec206b670aef249af8784f6ab1b943711 Mon Sep 17 00:00:00 2001 From: Scott Sadler Date: Thu, 23 Feb 2017 19:25:53 +0100 Subject: [PATCH 08/23] fix tests, temporarily disabling some tests that need to be re-written --- bigchaindb/core.py | 33 ++++++------ bigchaindb/pipelines/election.py | 3 +- bigchaindb/pipelines/vote.py | 3 +- bigchaindb/voting.py | 9 ++-- tests/db/test_bigchain_api.py | 43 ++------------- tests/pipelines/test_election.py | 4 +- tests/test_consensus.py | 40 -------------- tests/test_voting.py | 89 ++++++++++++++++++++++++++++++++ tests/web/test_statuses.py | 6 +-- 9 files changed, 124 insertions(+), 106 deletions(-) diff --git a/bigchaindb/core.py b/bigchaindb/core.py index d498a6d4..5186b7f2 100644 --- a/bigchaindb/core.py +++ b/bigchaindb/core.py @@ -486,12 +486,11 @@ class Bigchain(object): """ return self.consensus.validate_block(self, block) - def has_previous_vote(self, block_id, voters): + def has_previous_vote(self, block_id): """Check for previous votes from this node Args: block_id (str): the id of the block to check - voters (list(str)): the voters of the block to check Returns: bool: :const:`True` if this block already has a @@ -507,15 +506,14 @@ class Bigchain(object): if len(votes) > 1: raise exceptions.MultipleVotesError('Block {block_id} has {n_votes} votes from public key {me}' .format(block_id=block_id, n_votes=str(len(votes)), me=self.me)) - has_previous_vote = False - if votes: - if utils.verify_vote_signature(voters, votes[0]): - has_previous_vote = True - else: - raise exceptions.ImproperVoteError('Block {block_id} already has an incorrectly signed vote ' - 'from public key {me}'.format(block_id=block_id, me=self.me)) + if len(votes) < 1: + return False - return has_previous_vote + if self.consensus.voting.verify_vote_signature(votes[0]): + return True + else: + raise exceptions.ImproperVoteError('Block {block_id} already has an incorrectly signed vote ' + 'from public key {me}'.format(block_id=block_id, me=self.me)) def write_block(self, block): """Write a block to bigchain. @@ -618,12 +616,17 @@ class Bigchain(object): # XXX: should this return instaces of Block? return backend.query.get_unvoted_blocks(self.connection, self.me) - def block_election_status(self, block): - """Tally the votes on a block, and return the status: - valid, invalid, or undecided.""" + def block_election(self, block): + if type(block) != dict: + block = block.to_dict() votes = list(backend.query.get_votes_by_block_id(self.connection, - block.id)) + block['id'])) keyring = self.nodes_except_me + [self.me] result = self.consensus.voting.block_election(block, votes, keyring) # TODO: logging - return result['status'] + return result + + def block_election_status(self, block): + """Tally the votes on a block, and return the status: + valid, invalid, or undecided.""" + return self.block_election(block)['status'] diff --git a/bigchaindb/pipelines/election.py b/bigchaindb/pipelines/election.py index 850613a3..2e5efc3c 100644 --- a/bigchaindb/pipelines/election.py +++ b/bigchaindb/pipelines/election.py @@ -35,8 +35,7 @@ class Election: next_block = self.bigchain.get_block( next_vote['vote']['voting_for_block']) - block_status = self.bigchain.block_election_status(next_block['id'], - next_block['block']['voters']) + block_status = self.bigchain.block_election_status(next_block) if block_status == self.bigchain.BLOCK_INVALID: return Block.from_dict(next_block) diff --git a/bigchaindb/pipelines/vote.py b/bigchaindb/pipelines/vote.py index 8d4f4386..e055caac 100644 --- a/bigchaindb/pipelines/vote.py +++ b/bigchaindb/pipelines/vote.py @@ -57,8 +57,7 @@ class Vote: [([self.bigchain.me], 1)]) def validate_block(self, block): - if not self.bigchain.has_previous_vote(block['id'], - block['block']['voters']): + if not self.bigchain.has_previous_vote(block['id']): try: block = Block.from_dict(block) except (exceptions.InvalidHash): diff --git a/bigchaindb/voting.py b/bigchaindb/voting.py index 62eb27ee..bb39c517 100644 --- a/bigchaindb/voting.py +++ b/bigchaindb/voting.py @@ -28,11 +28,12 @@ class Voting: """ Calculate the election status of a block. """ - eligible_voters = set(block['voters']) & set(keyring) + eligible_voters = set(block['block']['voters']) & set(keyring) eligible_votes, ineligible_votes = \ cls.partition_eligible_votes(votes, eligible_voters) + n_voters = len(eligible_voters) results = cls.count_votes(eligible_votes) - results['status'] = cls.decide_votes(results['counts']) + results['status'] = cls.decide_votes(n_voters, **results['counts']) results['ineligible'] = ineligible_votes return results @@ -100,11 +101,13 @@ class Voting: else: n_invalid += 1 + n_prev = prev_blocks.most_common()[0][1] if prev_blocks else 0 + return { 'counts': { 'n_valid': n_valid, 'n_invalid': n_invalid, - 'n_agree_prev_block': prev_blocks.most_common()[0][1], + 'n_agree_prev_block': n_prev, }, 'cheat': cheat, 'malformed': malformed, diff --git a/tests/db/test_bigchain_api.py b/tests/db/test_bigchain_api.py index a530577b..2363f9e7 100644 --- a/tests/db/test_bigchain_api.py +++ b/tests/db/test_bigchain_api.py @@ -82,12 +82,12 @@ class TestBigchainApi(object): block = b.create_block([tx]) b.write_block(block) - assert b.has_previous_vote(block.id, block.voters) is False + assert b.has_previous_vote(block.id) is False vote = b.vote(block.id, b.get_last_voted_block().id, True) b.write_vote(vote) - assert b.has_previous_vote(block.id, block.voters) is True + assert b.has_previous_vote(block.id) is True @pytest.mark.genesis def test_get_spent_with_double_inclusion_detected(self, b, monkeypatch): @@ -428,43 +428,6 @@ class TestBigchainApi(object): assert retrieved_block_1 == retrieved_block_2 - @pytest.mark.genesis - def test_more_votes_than_voters(self, b): - from bigchaindb.common.exceptions import MultipleVotesError - - block_1 = dummy_block() - b.write_block(block_1) - # insert duplicate votes - vote_1 = b.vote(block_1.id, b.get_last_voted_block().id, True) - vote_2 = b.vote(block_1.id, b.get_last_voted_block().id, True) - vote_2['node_pubkey'] = 'aaaaaaa' - b.write_vote(vote_1) - b.write_vote(vote_2) - - with pytest.raises(MultipleVotesError) as excinfo: - b.block_election_status(block_1.id, block_1.voters) - assert excinfo.value.args[0] == 'Block {block_id} has {n_votes} votes cast, but only {n_voters} voters'\ - .format(block_id=block_1.id, n_votes=str(2), n_voters=str(1)) - - def test_multiple_votes_single_node(self, b, genesis_block): - from bigchaindb.common.exceptions import MultipleVotesError - - block_1 = dummy_block() - b.write_block(block_1) - # insert duplicate votes - for i in range(2): - b.write_vote(b.vote(block_1.id, genesis_block.id, True)) - - with pytest.raises(MultipleVotesError) as excinfo: - b.block_election_status(block_1.id, block_1.voters) - assert excinfo.value.args[0] == 'Block {block_id} has multiple votes ({n_votes}) from voting node {node_id}'\ - .format(block_id=block_1.id, n_votes=str(2), node_id=b.me) - - with pytest.raises(MultipleVotesError) as excinfo: - b.has_previous_vote(block_1.id, block_1.voters) - assert excinfo.value.args[0] == 'Block {block_id} has {n_votes} votes from public key {me}'\ - .format(block_id=block_1.id, n_votes=str(2), me=b.me) - @pytest.mark.genesis def test_improper_vote_error(selfs, b): from bigchaindb.common.exceptions import ImproperVoteError @@ -476,7 +439,7 @@ class TestBigchainApi(object): vote_1['signature'] = 'a' * 87 b.write_vote(vote_1) with pytest.raises(ImproperVoteError) as excinfo: - b.has_previous_vote(block_1.id, block_1.id) + b.has_previous_vote(block_1.id) assert excinfo.value.args[0] == 'Block {block_id} already has an incorrectly signed ' \ 'vote from public key {me}'.format(block_id=block_1.id, me=b.me) diff --git a/tests/pipelines/test_election.py b/tests/pipelines/test_election.py index 5cf6fc14..bb01b6d1 100644 --- a/tests/pipelines/test_election.py +++ b/tests/pipelines/test_election.py @@ -96,8 +96,10 @@ def test_check_for_quorum_valid(b, user_pk): for key_pair in key_pairs ] + keyring = e.bigchain.nodes_except_me = [key_pair[1] for key_pair in key_pairs] + # add voters to block and write - test_block.voters = [key_pair[1] for key_pair in key_pairs] + test_block.voters = keyring test_block = test_block.sign(b.me_private) b.write_block(test_block) diff --git a/tests/test_consensus.py b/tests/test_consensus.py index 7310f514..e69de29b 100644 --- a/tests/test_consensus.py +++ b/tests/test_consensus.py @@ -1,40 +0,0 @@ - -def test_verify_vote_passes(b, structurally_valid_vote): - from bigchaindb.consensus import BaseConsensusRules - from bigchaindb.common import crypto - from bigchaindb.common.utils import serialize - vote_body = structurally_valid_vote['vote'] - vote_data = serialize(vote_body) - signature = crypto.PrivateKey(b.me_private).sign(vote_data.encode()) - vote_signed = { - 'node_pubkey': b.me, - 'signature': signature.decode(), - 'vote': vote_body - } - assert BaseConsensusRules.verify_vote([b.me], vote_signed) - - -def test_verify_vote_fails_signature(b, structurally_valid_vote): - from bigchaindb.consensus import BaseConsensusRules - vote_body = structurally_valid_vote['vote'] - vote_signed = { - 'node_pubkey': b.me, - 'signature': 'a' * 86, - 'vote': vote_body - } - assert not BaseConsensusRules.verify_vote([b.me], vote_signed) - - -def test_verify_vote_fails_schema(b): - from bigchaindb.consensus import BaseConsensusRules - from bigchaindb.common import crypto - from bigchaindb.common.utils import serialize - vote_body = {} - vote_data = serialize(vote_body) - signature = crypto.PrivateKey(b.me_private).sign(vote_data.encode()) - vote_signed = { - 'node_pubkey': b.me, - 'signature': signature.decode(), - 'vote': vote_body - } - assert not BaseConsensusRules.verify_vote([b.me], vote_signed) diff --git a/tests/test_voting.py b/tests/test_voting.py index 2d7b723f..07f640fc 100644 --- a/tests/test_voting.py +++ b/tests/test_voting.py @@ -82,3 +82,92 @@ def test_decide_votes_checks_arguments(): with pytest.raises(ValueError): Voting.decide_votes(n_voters=1, n_valid=0, n_invalid=0, n_agree_prev_block=2) + + + +################################################################################ + +# DEBT + + + +def _test_verify_vote_passes(b, structurally_valid_vote): + from bigchaindb.consensus import BaseConsensusRules + from bigchaindb.common import crypto + from bigchaindb.common.utils import serialize + vote_body = structurally_valid_vote['vote'] + vote_data = serialize(vote_body) + signature = crypto.PrivateKey(b.me_private).sign(vote_data.encode()) + vote_signed = { + 'node_pubkey': b.me, + 'signature': signature.decode(), + 'vote': vote_body + } + assert BaseConsensusRules.verify_vote([b.me], vote_signed) + + +def _test_verify_vote_fails_signature(b, structurally_valid_vote): + from bigchaindb.consensus import BaseConsensusRules + vote_body = structurally_valid_vote['vote'] + vote_signed = { + 'node_pubkey': b.me, + 'signature': 'a' * 86, + 'vote': vote_body + } + assert not BaseConsensusRules.verify_vote([b.me], vote_signed) + + +def _test_verify_vote_fails_schema(b): + from bigchaindb.consensus import BaseConsensusRules + from bigchaindb.common import crypto + from bigchaindb.common.utils import serialize + vote_body = {} + vote_data = serialize(vote_body) + signature = crypto.PrivateKey(b.me_private).sign(vote_data.encode()) + vote_signed = { + 'node_pubkey': b.me, + 'signature': signature.decode(), + 'vote': vote_body + } + assert not BaseConsensusRules.verify_vote([b.me], vote_signed) + + +""" + @pytest.mark.genesis + def test_more_votes_than_voters(self, b): + from bigchaindb.common.exceptions import MultipleVotesError + + block_1 = dummy_block() + b.write_block(block_1) + # insert duplicate votes + vote_1 = b.vote(block_1.id, b.get_last_voted_block().id, True) + vote_2 = b.vote(block_1.id, b.get_last_voted_block().id, True) + vote_2['node_pubkey'] = 'aaaaaaa' + b.write_vote(vote_1) + b.write_vote(vote_2) + + with pytest.raises(MultipleVotesError) as excinfo: + b.block_election_status(block_1.id, block_1.voters) + assert excinfo.value.args[0] == 'Block {block_id} has {n_votes} votes cast, but only {n_voters} voters'\ + .format(block_id=block_1.id, n_votes=str(2), n_voters=str(1)) + + def test_multiple_votes_single_node(self, b, genesis_block): + from bigchaindb.common.exceptions import MultipleVotesError + + block_1 = dummy_block() + b.write_block(block_1) + # insert duplicate votes + for i in range(2): + b.write_vote(b.vote(block_1.id, genesis_block.id, True)) + + with pytest.raises(MultipleVotesError) as excinfo: + b.block_election_status(block_1.id, block_1.voters) + assert excinfo.value.args[0] == 'Block {block_id} has multiple votes ({n_votes}) from voting node {node_id}'\ + .format(block_id=block_1.id, n_votes=str(2), node_id=b.me) + + with pytest.raises(MultipleVotesError) as excinfo: + b.has_previous_vote(block_1.id) + assert excinfo.value.args[0] == 'Block {block_id} has {n_votes} votes from public key {me}'\ + .format(block_id=block_1.id, n_votes=str(2), me=b.me) + +""" diff --git a/tests/web/test_statuses.py b/tests/web/test_statuses.py index af9d09d3..716cc0d2 100644 --- a/tests/web/test_statuses.py +++ b/tests/web/test_statuses.py @@ -30,7 +30,7 @@ def test_get_block_status_endpoint_undecided(b, client): block = b.create_block([tx]) b.write_block(block) - status = b.block_election_status(block.id, block.voters) + status = b.block_election_status(block) res = client.get(STATUSES_ENDPOINT + '?block_id=' + block.id) assert status == res.json['status'] @@ -51,7 +51,7 @@ def test_get_block_status_endpoint_valid(b, client): vote = b.vote(block.id, b.get_last_voted_block().id, True) b.write_vote(vote) - status = b.block_election_status(block.id, block.voters) + status = b.block_election_status(block) res = client.get(STATUSES_ENDPOINT + '?block_id=' + block.id) assert status == res.json['status'] @@ -72,7 +72,7 @@ def test_get_block_status_endpoint_invalid(b, client): vote = b.vote(block.id, b.get_last_voted_block().id, False) b.write_vote(vote) - status = b.block_election_status(block.id, block.voters) + status = b.block_election_status(block) res = client.get(STATUSES_ENDPOINT + '?block_id=' + block.id) assert status == res.json['status'] From 1ff84bd670ad2b166cc7d57dbb2df2be2689285f Mon Sep 17 00:00:00 2001 From: Scott Sadler Date: Thu, 23 Feb 2017 21:10:12 +0100 Subject: [PATCH 09/23] test_partition_eligible_votes --- bigchaindb/voting.py | 7 +++---- tests/test_voting.py | 25 ++++++++++++++++++------- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/bigchaindb/voting.py b/bigchaindb/voting.py index bb39c517..7962eec4 100644 --- a/bigchaindb/voting.py +++ b/bigchaindb/voting.py @@ -49,12 +49,11 @@ class Voting: voter_eligible = vote.get('node_pubkey') in eligible_voters if voter_eligible: try: - cls.verify_vote_signature(vote) + if cls.verify_vote_signature(vote): + eligible.append(vote) + continue except ValueError: pass - else: - eligible.append(vote) - continue ineligible.append(vote) return eligible, ineligible diff --git a/tests/test_voting.py b/tests/test_voting.py index 07f640fc..5f1a1069 100644 --- a/tests/test_voting.py +++ b/tests/test_voting.py @@ -9,15 +9,26 @@ from bigchaindb.voting import Voting, INVALID, VALID, UNDECIDED # Tests for checking vote eligibility -@patch('bigchaindb.voting.Voting.verify_vote_signature') -def test_partition_eligible_votes(_): - nodes = list(map(Bigchain, 'abc')) - votes = [n.vote('block', 'a', True) for n in nodes] +def test_partition_eligible_votes(): + class TestVoting(Voting): + @classmethod + def verify_vote_signature(cls, vote): + if vote['node_pubkey'] == 'invalid sig': + return False + if vote['node_pubkey'] == 'value error': + raise ValueError() + return True - el, inel = Voting.partition_eligible_votes(votes, 'abc') + voters = ['valid', 'invalid sig', 'value error', 'not in set'] + votes = [{'node_pubkey': k} for k in voters] - assert el == votes - assert inel == [] + el, inel = TestVoting.partition_eligible_votes(votes, voters[:-1]) + assert el == [votes[0]] + assert inel == votes[1:] + + +################################################################################ +# Test vote counting @patch('bigchaindb.voting.Voting.verify_vote_schema') From e88c98a695d12481eedd399647bc54da5ec96721 Mon Sep 17 00:00:00 2001 From: Scott Sadler Date: Thu, 23 Feb 2017 21:39:38 +0100 Subject: [PATCH 10/23] test_count_votes --- bigchaindb/voting.py | 2 +- tests/test_voting.py | 27 +++++++++++++++++++-------- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/bigchaindb/voting.py b/bigchaindb/voting.py index 7962eec4..c5482e92 100644 --- a/bigchaindb/voting.py +++ b/bigchaindb/voting.py @@ -94,8 +94,8 @@ class Voting: n_invalid += 1 continue - prev_blocks[vote['vote']['previous_block']] += 1 if vote['vote']['is_block_valid']: + prev_blocks[vote['vote']['previous_block']] += 1 n_valid += 1 else: n_invalid += 1 diff --git a/tests/test_voting.py b/tests/test_voting.py index 5f1a1069..e70addf1 100644 --- a/tests/test_voting.py +++ b/tests/test_voting.py @@ -31,16 +31,27 @@ def test_partition_eligible_votes(): # Test vote counting -@patch('bigchaindb.voting.Voting.verify_vote_schema') -def test_count_votes(_): - nodes = list(map(Bigchain, 'abc')) +def test_count_votes(): + class TestVoting(Voting): + @classmethod + def verify_vote_schema(cls, vote): + return vote['node_pubkey'] != 'malformed' - votes = [n.vote('block', 'a', True) for n in nodes] + voters = ['cheat', 'cheat', 'says invalid', 'malformed'] + voters += ['kosher' + str(i) for i in range(10)] - assert Voting.count_votes(votes)['counts'] == { - 'n_valid': 3, - 'n_invalid': 0, - 'n_agree_prev_block': 3 + votes = [Bigchain(v).vote('block', 'a', True) for v in voters] + votes[2]['vote']['is_block_valid'] = False + votes[-1]['vote']['previous_block'] = 'z' + + assert TestVoting.count_votes(votes) == { + 'counts': { + 'n_valid': 10, + 'n_invalid': 3, + 'n_agree_prev_block': 9 + }, + 'cheat': [votes[:2]], + 'malformed': [votes[3]], } From c44c9d0282ae7c85b67f1dfb5517085e9e28593c Mon Sep 17 00:00:00 2001 From: Scott Sadler Date: Thu, 23 Feb 2017 21:52:59 +0100 Subject: [PATCH 11/23] tests for vote schema and signature validation --- bigchaindb/voting.py | 2 +- tests/test_voting.py | 62 ++++++++++++++------------------------------ 2 files changed, 21 insertions(+), 43 deletions(-) diff --git a/bigchaindb/voting.py b/bigchaindb/voting.py index c5482e92..21ce8195 100644 --- a/bigchaindb/voting.py +++ b/bigchaindb/voting.py @@ -170,5 +170,5 @@ class Voting: try: validate_vote_schema(vote) return True - except SchemaValidationError: + except SchemaValidationError as e: return False diff --git a/tests/test_voting.py b/tests/test_voting.py index e70addf1..cea34de0 100644 --- a/tests/test_voting.py +++ b/tests/test_voting.py @@ -106,52 +106,30 @@ def test_decide_votes_checks_arguments(): n_agree_prev_block=2) +################################################################################ +# Tests for vote signature + + +def test_verify_vote_signature_passes(b): + vote = b.vote('block', 'a', True) + assert Voting.verify_vote_signature(vote) + + +def test_verify_vote_signature_fails(b): + vote = b.vote('block', 'a', True) + vote['signature'] = '' + assert not Voting.verify_vote_signature(vote) + ################################################################################ - -# DEBT +# Tests for vote schema - -def _test_verify_vote_passes(b, structurally_valid_vote): - from bigchaindb.consensus import BaseConsensusRules - from bigchaindb.common import crypto - from bigchaindb.common.utils import serialize - vote_body = structurally_valid_vote['vote'] - vote_data = serialize(vote_body) - signature = crypto.PrivateKey(b.me_private).sign(vote_data.encode()) - vote_signed = { - 'node_pubkey': b.me, - 'signature': signature.decode(), - 'vote': vote_body - } - assert BaseConsensusRules.verify_vote([b.me], vote_signed) - - -def _test_verify_vote_fails_signature(b, structurally_valid_vote): - from bigchaindb.consensus import BaseConsensusRules - vote_body = structurally_valid_vote['vote'] - vote_signed = { - 'node_pubkey': b.me, - 'signature': 'a' * 86, - 'vote': vote_body - } - assert not BaseConsensusRules.verify_vote([b.me], vote_signed) - - -def _test_verify_vote_fails_schema(b): - from bigchaindb.consensus import BaseConsensusRules - from bigchaindb.common import crypto - from bigchaindb.common.utils import serialize - vote_body = {} - vote_data = serialize(vote_body) - signature = crypto.PrivateKey(b.me_private).sign(vote_data.encode()) - vote_signed = { - 'node_pubkey': b.me, - 'signature': signature.decode(), - 'vote': vote_body - } - assert not BaseConsensusRules.verify_vote([b.me], vote_signed) +def test_verify_vote_schema(b): + vote = b.vote('b' * 64, 'a' * 64, True) + assert Voting.verify_vote_schema(vote) + vote = b.vote('b', 'a', True) + assert not Voting.verify_vote_schema(vote) """ From e1312b88a396c6de610e325891f77da5e4e9774b Mon Sep 17 00:00:00 2001 From: Scott Sadler Date: Fri, 24 Feb 2017 10:04:24 +0100 Subject: [PATCH 12/23] Voting uses BigchainDBCritical --- bigchaindb/voting.py | 3 ++- tests/test_voting.py | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/bigchaindb/voting.py b/bigchaindb/voting.py index 21ce8195..f8511644 100644 --- a/bigchaindb/voting.py +++ b/bigchaindb/voting.py @@ -1,4 +1,5 @@ import collections +from bigchaindb.backend.exceptions import BigchainDBCritical from bigchaindb.common.schema import SchemaValidationError, validate_vote_schema from bigchaindb.common.utils import serialize from bigchaindb.common.crypto import PublicKey @@ -126,7 +127,7 @@ class Voting: # Check insane cases. This is basic, not exhaustive. if n_valid + n_invalid > n_voters or n_agree_prev_block > n_voters: - raise ValueError('Arguments not sane: %s' % { + raise BigchainDBCritical('Arguments not sane: %s' % { 'n_voters': n_voters, 'n_valid': n_valid, 'n_invalid': n_invalid, diff --git a/tests/test_voting.py b/tests/test_voting.py index cea34de0..13822d6d 100644 --- a/tests/test_voting.py +++ b/tests/test_voting.py @@ -1,6 +1,6 @@ import pytest -from unittest.mock import patch +from bigchaindb.backend.exceptions import BigchainDBCritical from bigchaindb.core import Bigchain from bigchaindb.voting import Voting, INVALID, VALID, UNDECIDED @@ -95,13 +95,13 @@ def test_decide_votes_invalid(kwargs): def test_decide_votes_checks_arguments(): - with pytest.raises(ValueError): + with pytest.raises(BigchainDBCritical): Voting.decide_votes(n_voters=1, n_valid=2, n_invalid=0, n_agree_prev_block=0) - with pytest.raises(ValueError): + with pytest.raises(BigchainDBCritical): Voting.decide_votes(n_voters=1, n_valid=0, n_invalid=2, n_agree_prev_block=0) - with pytest.raises(ValueError): + with pytest.raises(BigchainDBCritical): Voting.decide_votes(n_voters=1, n_valid=0, n_invalid=0, n_agree_prev_block=2) From ae8367c6c7f02b545a74574f746d626fb961dc48 Mon Sep 17 00:00:00 2001 From: Scott Sadler Date: Fri, 24 Feb 2017 13:27:48 +0100 Subject: [PATCH 13/23] log election results in election pipeline --- bigchaindb/core.py | 1 - bigchaindb/pipelines/election.py | 25 +++++++++++++++++++++---- bigchaindb/voting.py | 1 + tests/test_voting.py | 3 ++- 4 files changed, 24 insertions(+), 6 deletions(-) diff --git a/bigchaindb/core.py b/bigchaindb/core.py index 5186b7f2..1f82fd78 100644 --- a/bigchaindb/core.py +++ b/bigchaindb/core.py @@ -623,7 +623,6 @@ class Bigchain(object): block['id'])) keyring = self.nodes_except_me + [self.me] result = self.consensus.voting.block_election(block, votes, keyring) - # TODO: logging return result def block_election_status(self, block): diff --git a/bigchaindb/pipelines/election.py b/bigchaindb/pipelines/election.py index 2e5efc3c..8e080183 100644 --- a/bigchaindb/pipelines/election.py +++ b/bigchaindb/pipelines/election.py @@ -16,6 +16,7 @@ from bigchaindb import Bigchain logger = logging.getLogger(__name__) +logger_results = logging.getLogger('pipeline.election.results') class Election: @@ -32,13 +33,29 @@ class Election: next_vote: The next vote. """ - next_block = self.bigchain.get_block( - next_vote['vote']['voting_for_block']) + try: + block_id = next_vote['vote']['voting_for_block'] + node = next_vote['node_pubkey'] + except IndexError: + return - block_status = self.bigchain.block_election_status(next_block) - if block_status == self.bigchain.BLOCK_INVALID: + next_block = self.bigchain.get_block(block_id) + + result = self.bigchain.block_election_status(next_block) + if result['status'] == self.bigchain.BLOCK_INVALID: return Block.from_dict(next_block) + # Log the result + if result['status'] != self.bigchain.BLOCK_UNDECIDED: + msg = 'node:%s block:%s status:%s' % \ + (node, block_id, result['status']) + # Extra data can be accessed via the log formatter. + # See logging.dictConfig. + logger_results.debug(msg, extra={ + 'current_vote': next_vote, + 'election_result': result, + }) + def requeue_transactions(self, invalid_block): """ Liquidates transactions from invalid blocks so they can be processed again diff --git a/bigchaindb/voting.py b/bigchaindb/voting.py index f8511644..3b36303f 100644 --- a/bigchaindb/voting.py +++ b/bigchaindb/voting.py @@ -34,6 +34,7 @@ class Voting: cls.partition_eligible_votes(votes, eligible_voters) n_voters = len(eligible_voters) results = cls.count_votes(eligible_votes) + results['block_id'] = block['id'] results['status'] = cls.decide_votes(n_voters, **results['counts']) results['ineligible'] = ineligible_votes return results diff --git a/tests/test_voting.py b/tests/test_voting.py index ea61fa6a..8fa72ca2 100644 --- a/tests/test_voting.py +++ b/tests/test_voting.py @@ -149,7 +149,7 @@ def test_block_election(b): return True keyring = 'abc' - block = {'block': {'voters': 'ab'}} + block = {'id': 'xyz', 'block': {'voters': 'ab'}} votes = [{ 'node_pubkey': c, 'vote': {'is_block_valid': True, 'previous_block': 'a'} @@ -157,6 +157,7 @@ def test_block_election(b): assert TestVoting.block_election(block, votes, keyring) == { 'status': VALID, + 'block_id': 'xyz', 'counts': {'n_agree_prev_block': 2, 'n_valid': 2, 'n_invalid': 0}, 'ineligible': [votes[-1]], 'cheat': [], From 6ab1089bda074a8d79f2e039cab4ade5255abf12 Mon Sep 17 00:00:00 2001 From: Scott Sadler Date: Sat, 25 Feb 2017 14:18:06 +0100 Subject: [PATCH 14/23] voting cleanup --- bigchaindb/voting.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bigchaindb/voting.py b/bigchaindb/voting.py index b86797d2..837621b1 100644 --- a/bigchaindb/voting.py +++ b/bigchaindb/voting.py @@ -12,14 +12,14 @@ UNDECIDED = 'undecided' class Voting: """ - Everything to do with creating and checking votes. + Everything to do with verifying and counting votes for block election. All functions in this class should be referentially transparent, that is, they always give the same output for a given input. This makes it easier to test. This also means no logging! Assumptions regarding data: - * Vote is a dictionary, but it is not assumed that any properties are. + * Vote is a dictionary, but no assumptions are made on it's properties. * Everything else is assumed to be structurally correct, otherwise errors may be thrown. """ @@ -30,9 +30,9 @@ class Voting: Calculate the election status of a block. """ eligible_voters = set(block['block']['voters']) & set(keyring) + n_voters = len(eligible_voters) eligible_votes, ineligible_votes = \ cls.partition_eligible_votes(votes, eligible_voters) - n_voters = len(eligible_voters) results = cls.count_votes(eligible_votes) results['block_id'] = block['id'] results['status'] = cls.decide_votes(n_voters, **results['counts']) From 9bdc8ca3415c09321991d5a207dcc53d083c7d9a Mon Sep 17 00:00:00 2001 From: Scott Sadler Date: Mon, 27 Feb 2017 16:23:46 +0100 Subject: [PATCH 15/23] fix in election pipeline --- bigchaindb/pipelines/election.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bigchaindb/pipelines/election.py b/bigchaindb/pipelines/election.py index 8e080183..fe4fbc68 100644 --- a/bigchaindb/pipelines/election.py +++ b/bigchaindb/pipelines/election.py @@ -41,7 +41,7 @@ class Election: next_block = self.bigchain.get_block(block_id) - result = self.bigchain.block_election_status(next_block) + result = self.bigchain.block_election(next_block) if result['status'] == self.bigchain.BLOCK_INVALID: return Block.from_dict(next_block) From ebeb94f35a0975e7733ad522ae2714910822423d Mon Sep 17 00:00:00 2001 From: Scott Sadler Date: Mon, 27 Feb 2017 16:25:29 +0100 Subject: [PATCH 16/23] cleanup has_previous_vote --- bigchaindb/common/exceptions.py | 8 -------- bigchaindb/core.py | 18 ++---------------- tests/db/test_bigchain_api.py | 15 --------------- 3 files changed, 2 insertions(+), 39 deletions(-) diff --git a/bigchaindb/common/exceptions.py b/bigchaindb/common/exceptions.py index 60340492..1b869e5c 100644 --- a/bigchaindb/common/exceptions.py +++ b/bigchaindb/common/exceptions.py @@ -62,14 +62,6 @@ class StartupError(BigchainDBError): """Raised when there is an error starting up the system""" -class ImproperVoteError(BigchainDBError): - """Raised if a vote is not constructed correctly, or signed incorrectly""" - - -class MultipleVotesError(BigchainDBError): - """Raised if a voter has voted more than once""" - - class GenesisBlockAlreadyExistsError(BigchainDBError): """Raised when trying to create the already existing genesis block""" diff --git a/bigchaindb/core.py b/bigchaindb/core.py index 1f82fd78..e4419bb4 100644 --- a/bigchaindb/core.py +++ b/bigchaindb/core.py @@ -496,24 +496,10 @@ class Bigchain(object): bool: :const:`True` if this block already has a valid vote from this node, :const:`False` otherwise. - Raises: - ImproperVoteError: If there is already a vote, - but the vote is invalid. - """ votes = list(backend.query.get_votes_by_block_id_and_voter(self.connection, block_id, self.me)) - - if len(votes) > 1: - raise exceptions.MultipleVotesError('Block {block_id} has {n_votes} votes from public key {me}' - .format(block_id=block_id, n_votes=str(len(votes)), me=self.me)) - if len(votes) < 1: - return False - - if self.consensus.voting.verify_vote_signature(votes[0]): - return True - else: - raise exceptions.ImproperVoteError('Block {block_id} already has an incorrectly signed vote ' - 'from public key {me}'.format(block_id=block_id, me=self.me)) + el, _ = self.consensus.voting.partition_eligible_votes(votes, [self.me]) + return bool(el) def write_block(self, block): """Write a block to bigchain. diff --git a/tests/db/test_bigchain_api.py b/tests/db/test_bigchain_api.py index 2363f9e7..5f5aa5c5 100644 --- a/tests/db/test_bigchain_api.py +++ b/tests/db/test_bigchain_api.py @@ -428,21 +428,6 @@ class TestBigchainApi(object): assert retrieved_block_1 == retrieved_block_2 - @pytest.mark.genesis - def test_improper_vote_error(selfs, b): - from bigchaindb.common.exceptions import ImproperVoteError - - block_1 = dummy_block() - b.write_block(block_1) - vote_1 = b.vote(block_1.id, b.get_last_voted_block().id, True) - # mangle the signature - vote_1['signature'] = 'a' * 87 - b.write_vote(vote_1) - with pytest.raises(ImproperVoteError) as excinfo: - b.has_previous_vote(block_1.id) - assert excinfo.value.args[0] == 'Block {block_id} already has an incorrectly signed ' \ - 'vote from public key {me}'.format(block_id=block_1.id, me=b.me) - @pytest.mark.usefixtures('inputs') def test_assign_transaction_one_node(self, b, user_pk, user_sk): from bigchaindb.backend import query From c993f954e088e3ccd88c156e8bab570286c1637e Mon Sep 17 00:00:00 2001 From: Scott Sadler Date: Mon, 27 Feb 2017 20:56:01 +0100 Subject: [PATCH 17/23] wip --- bigchaindb/core.py | 25 ++++++++----------------- bigchaindb/models.py | 6 ++++-- tests/pipelines/test_election.py | 16 ++++++++-------- tests/test_models.py | 3 +-- 4 files changed, 21 insertions(+), 29 deletions(-) diff --git a/bigchaindb/core.py b/bigchaindb/core.py index e4419bb4..61ea5e8c 100644 --- a/bigchaindb/core.py +++ b/bigchaindb/core.py @@ -69,6 +69,9 @@ class Bigchain(object): if not self.me or not self.me_private: raise exceptions.KeypairNotFoundException() + federation = property(lambda self: set(self.nodes_except_me + [self.me])) + """ Set of federation member public keys """ + def write_transaction(self, signed_transaction): """Write the transaction to bigchain. @@ -107,19 +110,8 @@ class Bigchain(object): dict: database response or None if no reassignment is possible """ - if self.nodes_except_me: - try: - federation_nodes = self.nodes_except_me + [self.me] - index_current_assignee = federation_nodes.index(transaction['assignee']) - new_assignee = random.choice(federation_nodes[:index_current_assignee] + - federation_nodes[index_current_assignee + 1:]) - except ValueError: - # current assignee not in federation - new_assignee = random.choice(self.nodes_except_me) - - else: - # There is no other node to assign to - new_assignee = self.me + other_nodes = self.federation.difference([transaction['assignee']]) + new_assignee = random.choice(other_nodes) if other_nodes else self.me return backend.query.update_transaction( self.connection, transaction['id'], @@ -467,7 +459,7 @@ class Bigchain(object): raise exceptions.OperationError('Empty block creation is not ' 'allowed') - voters = self.nodes_except_me + [self.me] + voters = list(self.federation) block = Block(validated_transactions, self.me, gen_timestamp(), voters) block = block.sign(self.me_private) @@ -607,9 +599,8 @@ class Bigchain(object): block = block.to_dict() votes = list(backend.query.get_votes_by_block_id(self.connection, block['id'])) - keyring = self.nodes_except_me + [self.me] - result = self.consensus.voting.block_election(block, votes, keyring) - return result + return self.consensus.voting.block_election(block, votes, + self.federation) def block_election_status(self, block): """Tally the votes on a block, and return the status: diff --git a/bigchaindb/models.py b/bigchaindb/models.py index ee7efe8f..2d16800f 100644 --- a/bigchaindb/models.py +++ b/bigchaindb/models.py @@ -236,10 +236,12 @@ class Block(object): InvalidSignature: If a Block's signature is invalid. """ # Check if the block was created by a federation node - possible_voters = (bigchain.nodes_except_me + [bigchain.me]) - if self.node_pubkey not in possible_voters: + if self.node_pubkey not in bigchain.federation: raise OperationError('Only federation nodes can create blocks') + if set(self.voters) != bigchain.federation: + raise OperationError('Block voters differs from server keyring') + # Check that the signature is valid if not self.is_signature_valid(): raise InvalidSignature('Invalid block signature') diff --git a/tests/pipelines/test_election.py b/tests/pipelines/test_election.py index bb01b6d1..e7491656 100644 --- a/tests/pipelines/test_election.py +++ b/tests/pipelines/test_election.py @@ -83,12 +83,6 @@ def test_check_for_quorum_invalid_prev_node(b, user_pk): def test_check_for_quorum_valid(b, user_pk): from bigchaindb.models import Transaction - e = election.Election() - - # create blocks with transactions - tx1 = Transaction.create([b.me], [([user_pk], 1)]) - test_block = b.create_block([tx1]) - # simulate a federation with four voters key_pairs = [crypto.generate_key_pair() for _ in range(4)] test_federation = [ @@ -96,10 +90,13 @@ def test_check_for_quorum_valid(b, user_pk): for key_pair in key_pairs ] - keyring = e.bigchain.nodes_except_me = [key_pair[1] for key_pair in key_pairs] + b.nodes_except_me = [key_pair[1] for key_pair in key_pairs] + + # create blocks with transactions + tx1 = Transaction.create([b.me], [([user_pk], 1)]) + test_block = b.create_block([tx1]) # add voters to block and write - test_block.voters = keyring test_block = test_block.sign(b.me_private) b.write_block(test_block) @@ -110,6 +107,9 @@ def test_check_for_quorum_valid(b, user_pk): for vote in votes: b.write_vote(vote) + e = election.Election() + e.bigchain = b + # since this block is valid, should go nowhere assert e.check_for_quorum(votes[-1]) is None diff --git a/tests/test_models.py b/tests/test_models.py index 8de3a6c2..0b8be0b0 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -115,13 +115,12 @@ class TestBlockModel(object): transactions = [Transaction.create([b.me], [([b.me], 1)])] timestamp = gen_timestamp() - voters = ['Qaaa', 'Qbbb'] block = { 'timestamp': timestamp, 'transactions': [tx.to_dict() for tx in transactions], 'node_pubkey': b.me, - 'voters': voters, + 'voters': list(b.federation), } block_body = { From d3bd9d0300120c7a99269e958e2025ba560e1538 Mon Sep 17 00:00:00 2001 From: Troy McConaghy Date: Tue, 7 Mar 2017 11:43:49 +0100 Subject: [PATCH 18/23] Server docs: Removed the old 'Topic Guides' section --- bigchaindb/README.md | 2 +- docs/server/source/index.rst | 1 - docs/server/source/topic-guides/index.rst | 12 ------------ docs/server/source/topic-guides/models.md | 6 ------ 4 files changed, 1 insertion(+), 20 deletions(-) delete mode 100644 docs/server/source/topic-guides/index.rst delete mode 100644 docs/server/source/topic-guides/models.md diff --git a/bigchaindb/README.md b/bigchaindb/README.md index dbb59a1e..cd177c85 100644 --- a/bigchaindb/README.md +++ b/bigchaindb/README.md @@ -12,7 +12,7 @@ The `Bigchain` class is defined here. Most operations outlined in the [whitepap ### [`models.py`](./models.py) -`Block`, `Transaction`, and `Asset` classes are defined here. The classes mirror the block and transaction structure from the [documentation](https://docs.bigchaindb.com/projects/server/en/latest/topic-guides/models.html), but also include methods for validation and signing. +`Block`, `Transaction`, and `Asset` classes are defined here. The classes mirror the block and transaction structure from the [documentation](https://docs.bigchaindb.com/projects/server/en/latest/data-models/index.html), but also include methods for validation and signing. ### [`consensus.py`](./consensus.py) diff --git a/docs/server/source/index.rst b/docs/server/source/index.rst index 7f85a228..6ac4b9f5 100644 --- a/docs/server/source/index.rst +++ b/docs/server/source/index.rst @@ -13,7 +13,6 @@ BigchainDB Server Documentation server-reference/index drivers-clients/index clusters-feds/index - topic-guides/index data-models/index schema/transaction schema/vote diff --git a/docs/server/source/topic-guides/index.rst b/docs/server/source/topic-guides/index.rst deleted file mode 100644 index 9386fe87..00000000 --- a/docs/server/source/topic-guides/index.rst +++ /dev/null @@ -1,12 +0,0 @@ -Topic Guides -============ - -.. note:: - - Most of the Topic Guides have been moved over to `the root BigchainDB project docs `_. - - -.. toctree:: - :maxdepth: 1 - - models diff --git a/docs/server/source/topic-guides/models.md b/docs/server/source/topic-guides/models.md deleted file mode 100644 index 7f993feb..00000000 --- a/docs/server/source/topic-guides/models.md +++ /dev/null @@ -1,6 +0,0 @@ -# The Transaction, Block and Vote Models - -This page about transaction concepts and data models was getting too big, so it was split into smaller pages. It will be deleted eventually, so update your links. Here's where you can find the new pages: - -* [Transaction Concepts](https://docs.bigchaindb.com/en/latest/transaction-concepts.html) -* [Data Models (all of them)](../data-models/index.html) From e011f50bc70318a19bd8abaa01c21c4e1b578baa Mon Sep 17 00:00:00 2001 From: Scott Sadler Date: Wed, 8 Mar 2017 17:33:35 +0100 Subject: [PATCH 19/23] remove stray test --- tests/test_core.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/tests/test_core.py b/tests/test_core.py index cd2b96d2..8e0a63fc 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -82,15 +82,6 @@ def test_get_blocks_status_containing_tx(monkeypatch): bigchain.get_blocks_status_containing_tx('txid') -@pytest.mark.parametrize('exists', (True, False)) -def test_transaction_exists(monkeypatch, exists): - from bigchaindb.core import Bigchain - monkeypatch.setattr( - 'bigchaindb.backend.query.has_transaction', lambda x, y: exists) - bigchain = Bigchain(public_key='pubkey', private_key='privkey') - assert bigchain.transaction_exists('txid') is exists - - def test_has_previous_vote(monkeypatch): from bigchaindb.core import Bigchain monkeypatch.setattr( From dc58466de3bd071fbf4190c7ebf577da652f8d4c Mon Sep 17 00:00:00 2001 From: Scott Sadler Date: Mon, 13 Mar 2017 13:35:03 +0100 Subject: [PATCH 20/23] Merge branch 'master' into voting-class-integration --- bigchaindb/backend/exceptions.py | 4 - bigchaindb/common/exceptions.py | 121 +++++++++++++++---------- bigchaindb/core.py | 40 ++------ bigchaindb/exceptions.py | 8 ++ bigchaindb/models.py | 76 +++++----------- bigchaindb/pipelines/block.py | 14 +-- bigchaindb/pipelines/vote.py | 10 +- bigchaindb/web/views/transactions.py | 27 +----- tests/assets/test_digital_assets.py | 5 +- tests/db/test_bigchain_api.py | 79 ++++++++++++---- tests/pipelines/test_block_creation.py | 27 ++---- tests/pipelines/test_stale_monitor.py | 8 +- tests/pipelines/test_vote.py | 16 +++- tests/test_core.py | 10 -- tests/test_models.py | 8 +- tests/web/test_transactions.py | 12 +-- 16 files changed, 226 insertions(+), 239 deletions(-) diff --git a/bigchaindb/backend/exceptions.py b/bigchaindb/backend/exceptions.py index 3b712b08..017e19e4 100644 --- a/bigchaindb/backend/exceptions.py +++ b/bigchaindb/backend/exceptions.py @@ -15,7 +15,3 @@ class OperationError(BackendError): class DuplicateKeyError(OperationError): """Exception raised when an insert fails because the key is not unique""" - - -class BigchainDBCritical(Exception): - """Unhandleable error that requires attention""" diff --git a/bigchaindb/common/exceptions.py b/bigchaindb/common/exceptions.py index 18a926b1..258001b8 100644 --- a/bigchaindb/common/exceptions.py +++ b/bigchaindb/common/exceptions.py @@ -7,44 +7,6 @@ class ConfigurationError(BigchainDBError): """Raised when there is a problem with server configuration""" -class OperationError(BigchainDBError): - """Raised when an operation cannot go through""" - - -class TransactionDoesNotExist(BigchainDBError): - """Raised if the transaction is not in the database""" - - -class TransactionOwnerError(BigchainDBError): - """Raised if a user tries to transfer a transaction they don't own""" - - -class DoubleSpend(BigchainDBError): - """Raised if a double spend is found""" - - -class ValidationError(BigchainDBError): - """Raised if there was an error in validation""" - - -class InvalidHash(ValidationError): - """Raised if there was an error checking the hash for a particular - operation""" - - -class SchemaValidationError(ValidationError): - """Raised if there was any error validating an object's schema""" - - -class InvalidSignature(BigchainDBError): - """Raised if there was an error checking the signature for a particular - operation""" - - -class DuplicateTransaction(ValidationError): - """Raised if a duplicated transaction is found""" - - class DatabaseAlreadyExists(BigchainDBError): """Raised when trying to create the database but the db is already there""" @@ -53,15 +15,6 @@ class DatabaseDoesNotExist(BigchainDBError): """Raised when trying to delete the database but the db is not there""" -class KeypairNotFoundException(BigchainDBError): - """Raised if operation cannot proceed because the keypair was not given""" - - -class KeypairMismatchException(BigchainDBError): - """Raised if the private key(s) provided for signing don't match any of the - current owner(s)""" - - class StartupError(BigchainDBError): """Raised when there is an error starting up the system""" @@ -74,14 +27,82 @@ class CyclicBlockchainError(BigchainDBError): """Raised when there is a cycle in the blockchain""" -class TransactionNotInValidBlock(BigchainDBError): +class KeypairNotFoundException(BigchainDBError): + """Raised if operation cannot proceed because the keypair was not given""" + + +class KeypairMismatchException(BigchainDBError): + """Raised if the private key(s) provided for signing don't match any of the + current owner(s)""" + + +class OperationError(BigchainDBError): + """Raised when an operation cannot go through""" + + +################################################################################ +# Validation errors +# +# All validation errors (which are handleable errors, not faults) should +# subclass ValidationError. However, where possible they should also have their +# own distinct type to differentiate them from other validation errors, +# especially for the purposes of testing. + + +class ValidationError(BigchainDBError): + """Raised if there was an error in validation""" + + +class DoubleSpend(ValidationError): + """Raised if a double spend is found""" + + +class InvalidHash(ValidationError): + """Raised if there was an error checking the hash for a particular + operation""" + + +class SchemaValidationError(ValidationError): + """Raised if there was any error validating an object's schema""" + + +class InvalidSignature(ValidationError): + """Raised if there was an error checking the signature for a particular + operation""" + + +class ImproperVoteError(ValidationError): + """Raised if a vote is not constructed correctly, or signed incorrectly""" + + +class MultipleVotesError(ValidationError): + """Raised if a voter has voted more than once""" + + +class TransactionNotInValidBlock(ValidationError): """Raised when a transfer transaction is attempting to fulfill the outputs of a transaction that is in an invalid or undecided block""" -class AssetIdMismatch(BigchainDBError): +class AssetIdMismatch(ValidationError): """Raised when multiple transaction inputs related to different assets""" -class AmountError(BigchainDBError): +class AmountError(ValidationError): """Raised when there is a problem with a transaction's output amounts""" + + +class InputDoesNotExist(ValidationError): + """Raised if a transaction input does not exist""" + + +class TransactionOwnerError(ValidationError): + """Raised if a user tries to transfer a transaction they don't own""" + + +class SybilError(ValidationError): + """If a block or vote comes from an unidentifiable node""" + + +class DuplicateTransaction(ValidationError): + """Raised if a duplicated transaction is found""" diff --git a/bigchaindb/core.py b/bigchaindb/core.py index 9ac024c1..a9143f33 100644 --- a/bigchaindb/core.py +++ b/bigchaindb/core.py @@ -1,6 +1,7 @@ import random from time import time +from bigchaindb import exceptions as core_exceptions from bigchaindb.common import crypto, exceptions from bigchaindb.common.utils import gen_timestamp, serialize from bigchaindb.common.transaction import TransactionLink @@ -8,7 +9,6 @@ from bigchaindb.common.transaction import TransactionLink import bigchaindb from bigchaindb import backend, config_utils, utils -from bigchaindb.backend import exceptions as backend_exceptions from bigchaindb.consensus import BaseConsensusRules from bigchaindb.models import Block, Transaction @@ -110,7 +110,9 @@ class Bigchain(object): dict: database response or None if no reassignment is possible """ - other_nodes = self.federation.difference([transaction['assignee']]) + other_nodes = tuple( + self.federation.difference([transaction['assignee']]) + ) new_assignee = random.choice(other_nodes) if other_nodes else self.me return backend.query.update_transaction( @@ -151,31 +153,6 @@ class Bigchain(object): return self.consensus.validate_transaction(self, transaction) - def is_valid_transaction(self, transaction): - """Check whether a transaction is valid or invalid. - - Similar to :meth:`~bigchaindb.Bigchain.validate_transaction` - but never raises an exception. It returns :obj:`False` if - the transaction is invalid. - - Args: - transaction (:Class:`~bigchaindb.models.Transaction`): transaction - to check. - - Returns: - The :class:`~bigchaindb.models.Transaction` instance if valid, - otherwise :obj:`False`. - """ - - try: - return self.validate_transaction(transaction) - except (ValueError, exceptions.OperationError, - exceptions.TransactionDoesNotExist, - exceptions.TransactionOwnerError, exceptions.DoubleSpend, - exceptions.InvalidHash, exceptions.InvalidSignature, - exceptions.TransactionNotInValidBlock, exceptions.AmountError): - return False - def is_new_transaction(self, txid, exclude_block_id=None): """ Return True if the transaction does not exist in any @@ -317,7 +294,7 @@ class Bigchain(object): if list(validity.values()).count(Bigchain.BLOCK_VALID) > 1: block_ids = str([block for block in validity if validity[block] == Bigchain.BLOCK_VALID]) - raise backend_exceptions.BigchainDBCritical( + raise core_exceptions.CriticalDoubleInclusion( 'Transaction {tx} is present in ' 'multiple valid blocks: {block_ids}' .format(tx=txid, block_ids=block_ids)) @@ -370,10 +347,9 @@ class Bigchain(object): if self.get_transaction(transaction['id']): num_valid_transactions += 1 if num_valid_transactions > 1: - raise exceptions.DoubleSpend(('`{}` was spent more than' - ' once. There is a problem' - ' with the chain') - .format(txid)) + raise core_exceptions.CriticalDoubleSpend( + '`{}` was spent more than once. There is a problem' + ' with the chain'.format(txid)) if num_valid_transactions: return Transaction.from_dict(transactions[0]) diff --git a/bigchaindb/exceptions.py b/bigchaindb/exceptions.py index d8a4cd73..336ce231 100644 --- a/bigchaindb/exceptions.py +++ b/bigchaindb/exceptions.py @@ -1,2 +1,10 @@ class BigchainDBError(Exception): """Base class for BigchainDB exceptions.""" + + +class CriticalDoubleSpend(BigchainDBError): + """Data integrity error that requires attention""" + + +class CriticalDoubleInclusion(BigchainDBError): + """Data integrity error that requires attention""" diff --git a/bigchaindb/models.py b/bigchaindb/models.py index f3a20ebf..bf6bafd5 100644 --- a/bigchaindb/models.py +++ b/bigchaindb/models.py @@ -1,9 +1,9 @@ from bigchaindb.common.crypto import hash_data, PublicKey, PrivateKey from bigchaindb.common.exceptions import (InvalidHash, InvalidSignature, - OperationError, DoubleSpend, - TransactionDoesNotExist, + DoubleSpend, InputDoesNotExist, TransactionNotInValidBlock, AssetIdMismatch, AmountError, + SybilError, ValidationError, DuplicateTransaction) from bigchaindb.common.transaction import Transaction from bigchaindb.common.utils import gen_timestamp, serialize @@ -23,19 +23,10 @@ class Transaction(Transaction): invalid. Raises: - OperationError: if the transaction operation is not supported - TransactionDoesNotExist: if the input of the transaction is not - found - TransactionNotInValidBlock: if the input of the transaction is not - in a valid block - TransactionOwnerError: if the new transaction is using an input it - doesn't own - DoubleSpend: if the transaction is a double spend - InvalidHash: if the hash of the transaction is wrong - InvalidSignature: if the signature of the transaction is wrong + ValidationError: If the transaction is invalid """ if len(self.inputs) == 0: - raise ValueError('Transaction contains no inputs') + raise ValidationError('Transaction contains no inputs') input_conditions = [] inputs_defined = all([input_.fulfills for input_ in self.inputs]) @@ -47,20 +38,20 @@ class Transaction(Transaction): if self.operation in (Transaction.CREATE, Transaction.GENESIS): # validate asset if self.asset['data'] is not None and not isinstance(self.asset['data'], dict): - raise TypeError(('`asset.data` must be a dict instance or ' - 'None for `CREATE` transactions')) + raise ValidationError(('`asset.data` must be a dict instance or ' + 'None for `CREATE` transactions')) # validate inputs if inputs_defined: - raise ValueError('A CREATE operation has no inputs') + raise ValidationError('A CREATE operation has no inputs') elif self.operation == Transaction.TRANSFER: # validate asset if not isinstance(self.asset['id'], str): - raise ValueError(('`asset.id` must be a string for ' - '`TRANSFER` transations')) + raise ValidationError('`asset.id` must be a string for ' + '`TRANSFER` transations') # check inputs if not inputs_defined: - raise ValueError('Only `CREATE` transactions can have null ' - 'inputs') + raise ValidationError('Only `CREATE` transactions can have ' + 'null inputs') # store the inputs so that we can check if the asset ids match input_txs = [] @@ -70,8 +61,8 @@ class Transaction(Transaction): get_transaction(input_txid, include_status=True) if input_tx is None: - raise TransactionDoesNotExist("input `{}` doesn't exist" - .format(input_txid)) + raise InputDoesNotExist("input `{}` doesn't exist" + .format(input_txid)) if status != bigchain.TX_VALID: raise TransactionNotInValidBlock( @@ -117,8 +108,8 @@ class Transaction(Transaction): else: allowed_operations = ', '.join(Transaction.ALLOWED_OPERATIONS) - raise TypeError('`operation`: `{}` must be either {}.' - .format(self.operation, allowed_operations)) + raise ValidationError('`operation`: `{}` must be either {}.' + .format(self.operation, allowed_operations)) if not self.inputs_valid(input_conditions): raise InvalidSignature('Transaction signature is invalid.') @@ -206,18 +197,8 @@ class Block(object): raised. Raises: - OperationError: If a non-federation node signed the Block. - InvalidSignature: If a Block's signature is invalid or if the - block contains a transaction with an invalid signature. - OperationError: if the transaction operation is not supported - TransactionDoesNotExist: if the input of the transaction is not - found - TransactionNotInValidBlock: if the input of the transaction is not - in a valid block - TransactionOwnerError: if the new transaction is using an input it - doesn't own - DoubleSpend: if the transaction is a double spend - InvalidHash: if the hash of the transaction is wrong + ValidationError: If the block or any transaction in the block does + not validate """ self._validate_block(bigchain) @@ -233,15 +214,14 @@ class Block(object): object. Raises: - OperationError: If a non-federation node signed the Block. - InvalidSignature: If a Block's signature is invalid. + ValidationError: If there is a problem with the block """ # Check if the block was created by a federation node if self.node_pubkey not in bigchain.federation: - raise OperationError('Only federation nodes can create blocks') + raise SybilError('Only federation nodes can create blocks') if set(self.voters) != bigchain.federation: - raise OperationError('Block voters differs from server keyring') + raise SybilError('Block voters differs from server keyring') # Check that the signature is valid if not self.is_signature_valid(): @@ -254,17 +234,7 @@ class Block(object): bigchain (Bigchain): an instantiated bigchaindb.Bigchain object. Raises: - OperationError: if the transaction operation is not supported - TransactionDoesNotExist: if the input of the transaction is not - found - TransactionNotInValidBlock: if the input of the transaction is not - in a valid block - TransactionOwnerError: if the new transaction is using an input it - doesn't own - DoubleSpend: if the transaction is a double spend - InvalidHash: if the hash of the transaction is wrong - InvalidSignature: if the signature of the transaction is wrong - DuplicateTransaction: If the block contains a duplicated TX + ValidationError: If an invalid transaction is found """ txids = [tx.id for tx in self.transactions] if len(txids) != len(set(txids)): @@ -349,10 +319,10 @@ class Block(object): dict: The Block as a dict. Raises: - OperationError: If the Block doesn't contain any transactions. + ValueError: If the Block doesn't contain any transactions. """ if len(self.transactions) == 0: - raise OperationError('Empty block creation is not allowed') + raise ValueError('Empty block creation is not allowed') block = { 'timestamp': self.timestamp, diff --git a/bigchaindb/pipelines/block.py b/bigchaindb/pipelines/block.py index c7d7ebc1..0fe327bb 100644 --- a/bigchaindb/pipelines/block.py +++ b/bigchaindb/pipelines/block.py @@ -13,8 +13,7 @@ import bigchaindb from bigchaindb import backend from bigchaindb.backend.changefeed import ChangeFeed from bigchaindb.models import Transaction -from bigchaindb.common.exceptions import (SchemaValidationError, InvalidHash, - InvalidSignature, AmountError) +from bigchaindb.common.exceptions import ValidationError from bigchaindb import Bigchain @@ -63,8 +62,7 @@ class BlockPipeline: """ try: tx = Transaction.from_dict(tx) - except (SchemaValidationError, InvalidHash, InvalidSignature, - AmountError): + except ValidationError: return None # If transaction is in any VALID or UNDECIDED block we @@ -74,12 +72,14 @@ class BlockPipeline: return None # If transaction is not valid it should not be included - if not self.bigchain.is_valid_transaction(tx): + try: + tx.validate(self.bigchain) + return tx + except ValidationError as e: + logger.warning('Invalid tx: %s', e) self.bigchain.delete_transaction(tx.id) return None - return tx - def create(self, tx, timeout=False): """Create a block. diff --git a/bigchaindb/pipelines/vote.py b/bigchaindb/pipelines/vote.py index 4dd8b77c..9664c520 100644 --- a/bigchaindb/pipelines/vote.py +++ b/bigchaindb/pipelines/vote.py @@ -60,7 +60,7 @@ class Vote: return block['id'], [self.invalid_dummy_tx] try: block._validate_block(self.bigchain) - except (exceptions.OperationError, exceptions.InvalidSignature): + except exceptions.ValidationError: # XXX: if a block is invalid we should skip the `validate_tx` # step, but since we are in a pipeline we cannot just jump to # another function. Hackish solution: generate an invalid @@ -104,7 +104,13 @@ class Vote: if not new: return False, block_id, num_tx - valid = bool(self.bigchain.is_valid_transaction(tx)) + try: + tx.validate(self.bigchain) + valid = True + except exceptions.ValidationError as e: + logger.warning('Invalid tx: %s', e) + valid = False + return valid, block_id, num_tx def vote(self, tx_validity, block_id, num_tx): diff --git a/bigchaindb/web/views/transactions.py b/bigchaindb/web/views/transactions.py index 7acaa279..925aed7a 100644 --- a/bigchaindb/web/views/transactions.py +++ b/bigchaindb/web/views/transactions.py @@ -9,20 +9,7 @@ import logging from flask import current_app, request from flask_restful import Resource, reqparse - -from bigchaindb.common.exceptions import ( - AmountError, - DoubleSpend, - InvalidHash, - InvalidSignature, - SchemaValidationError, - OperationError, - TransactionDoesNotExist, - TransactionOwnerError, - TransactionNotInValidBlock, - ValidationError, -) - +from bigchaindb.common.exceptions import SchemaValidationError, ValidationError from bigchaindb.models import Transaction from bigchaindb.web.views.base import make_error from bigchaindb.web.views import parameters @@ -84,7 +71,7 @@ class TransactionListApi(Resource): message='Invalid transaction schema: {}'.format( e.__cause__.message) ) - except (ValidationError, InvalidSignature) as e: + except ValidationError as e: return make_error( 400, 'Invalid transaction ({}): {}'.format(type(e).__name__, e) @@ -93,15 +80,7 @@ class TransactionListApi(Resource): with pool() as bigchain: try: bigchain.validate_transaction(tx_obj) - except (ValueError, - OperationError, - TransactionDoesNotExist, - TransactionOwnerError, - DoubleSpend, - InvalidHash, - InvalidSignature, - TransactionNotInValidBlock, - AmountError) as e: + except ValidationError as e: return make_error( 400, 'Invalid transaction ({}): {}'.format(type(e).__name__, e) diff --git a/tests/assets/test_digital_assets.py b/tests/assets/test_digital_assets.py index 1dc4764f..d44bc52c 100644 --- a/tests/assets/test_digital_assets.py +++ b/tests/assets/test_digital_assets.py @@ -1,3 +1,4 @@ +from bigchaindb.common.exceptions import ValidationError import pytest import random @@ -26,7 +27,7 @@ def test_validate_bad_asset_creation(b, user_pk): tx.asset['data'] = 'a' tx_signed = tx.sign([b.me_private]) - with pytest.raises(TypeError): + with pytest.raises(ValidationError): b.validate_transaction(tx_signed) @@ -108,4 +109,4 @@ def test_create_valid_divisible_asset(b, user_pk, user_sk): tx = Transaction.create([user_pk], [([user_pk], 2)]) tx_signed = tx.sign([user_sk]) - assert b.is_valid_transaction(tx_signed) + tx_signed.validate(b) diff --git a/tests/db/test_bigchain_api.py b/tests/db/test_bigchain_api.py index 7a4b1f94..c5c9f1ae 100644 --- a/tests/db/test_bigchain_api.py +++ b/tests/db/test_bigchain_api.py @@ -3,6 +3,8 @@ from time import sleep import pytest from unittest.mock import patch +from bigchaindb.common.exceptions import ValidationError + pytestmark = pytest.mark.bdb @@ -91,7 +93,7 @@ class TestBigchainApi(object): @pytest.mark.genesis def test_get_spent_with_double_inclusion_detected(self, b, monkeypatch): - from bigchaindb.backend.exceptions import BigchainDBCritical + from bigchaindb.exceptions import CriticalDoubleInclusion from bigchaindb.models import Transaction tx = Transaction.create([b.me], [([b.me], 1)]) @@ -121,12 +123,47 @@ class TestBigchainApi(object): vote = b.vote(block3.id, b.get_last_voted_block().id, True) b.write_vote(vote) - with pytest.raises(BigchainDBCritical): + with pytest.raises(CriticalDoubleInclusion): + b.get_spent(tx.id, 0) + + @pytest.mark.genesis + def test_get_spent_with_double_spend_detected(self, b, monkeypatch): + from bigchaindb.exceptions import CriticalDoubleSpend + from bigchaindb.models import Transaction + + tx = Transaction.create([b.me], [([b.me], 1)]) + tx = tx.sign([b.me_private]) + + monkeypatch.setattr('time.time', lambda: 1000000000) + block1 = b.create_block([tx]) + b.write_block(block1) + + monkeypatch.setattr('time.time', lambda: 1000000020) + transfer_tx = Transaction.transfer(tx.to_inputs(), [([b.me], 1)], + asset_id=tx.id) + transfer_tx = transfer_tx.sign([b.me_private]) + block2 = b.create_block([transfer_tx]) + b.write_block(block2) + + monkeypatch.setattr('time.time', lambda: 1000000030) + transfer_tx2 = Transaction.transfer(tx.to_inputs(), [([b.me], 2)], + asset_id=tx.id) + transfer_tx2 = transfer_tx2.sign([b.me_private]) + block3 = b.create_block([transfer_tx2]) + b.write_block(block3) + + # Vote both block2 and block3 valid + vote = b.vote(block2.id, b.get_last_voted_block().id, True) + b.write_vote(vote) + vote = b.vote(block3.id, b.get_last_voted_block().id, True) + b.write_vote(vote) + + with pytest.raises(CriticalDoubleSpend): b.get_spent(tx.id, 0) @pytest.mark.genesis def test_get_block_status_for_tx_with_double_inclusion(self, b, monkeypatch): - from bigchaindb.backend.exceptions import BigchainDBCritical + from bigchaindb.exceptions import CriticalDoubleInclusion from bigchaindb.models import Transaction tx = Transaction.create([b.me], [([b.me], 1)]) @@ -146,7 +183,7 @@ class TestBigchainApi(object): vote = b.vote(block2.id, b.get_last_voted_block().id, True) b.write_vote(vote) - with pytest.raises(BigchainDBCritical): + with pytest.raises(CriticalDoubleInclusion): b.get_blocks_status_containing_tx(tx.id) @pytest.mark.genesis @@ -478,7 +515,7 @@ class TestBigchainApi(object): @pytest.mark.usefixtures('inputs') def test_non_create_input_not_found(self, b, user_pk): from cryptoconditions import Ed25519Fulfillment - from bigchaindb.common.exceptions import TransactionDoesNotExist + from bigchaindb.common.exceptions import InputDoesNotExist from bigchaindb.common.transaction import Input, TransactionLink from bigchaindb.models import Transaction from bigchaindb import Bigchain @@ -490,7 +527,7 @@ class TestBigchainApi(object): tx = Transaction.transfer([input], [([user_pk], 1)], asset_id='mock_asset_link') - with pytest.raises(TransactionDoesNotExist): + with pytest.raises(InputDoesNotExist): tx.validate(Bigchain()) def test_count_backlog(self, b, user_pk): @@ -513,24 +550,24 @@ class TestTransactionValidation(object): # Manipulate input so that it has a `fulfills` defined even # though it shouldn't have one create_tx.inputs[0].fulfills = TransactionLink('abc', 0) - with pytest.raises(ValueError) as excinfo: + with pytest.raises(ValidationError) as excinfo: b.validate_transaction(create_tx) assert excinfo.value.args[0] == 'A CREATE operation has no inputs' def test_transfer_operation_no_inputs(self, b, user_pk, signed_transfer_tx): signed_transfer_tx.inputs[0].fulfills = None - with pytest.raises(ValueError) as excinfo: + with pytest.raises(ValidationError) as excinfo: b.validate_transaction(signed_transfer_tx) assert excinfo.value.args[0] == 'Only `CREATE` transactions can have null inputs' def test_non_create_input_not_found(self, b, user_pk, signed_transfer_tx): - from bigchaindb.common.exceptions import TransactionDoesNotExist + from bigchaindb.common.exceptions import InputDoesNotExist from bigchaindb.common.transaction import TransactionLink signed_transfer_tx.inputs[0].fulfills = TransactionLink('c', 0) - with pytest.raises(TransactionDoesNotExist): + with pytest.raises(InputDoesNotExist): b.validate_transaction(signed_transfer_tx) @pytest.mark.usefixtures('inputs') @@ -689,7 +726,7 @@ class TestBlockValidation(object): b.validate_block(block) def test_invalid_node_pubkey(self, b): - from bigchaindb.common.exceptions import OperationError + from bigchaindb.common.exceptions import SybilError from bigchaindb.common import crypto # blocks can only be created by a federation node @@ -706,8 +743,8 @@ class TestBlockValidation(object): # from a non federation node block = block.sign(tmp_sk) - # check that validate_block raises an OperationError - with pytest.raises(OperationError): + # check that validate_block raises an SybilError + with pytest.raises(SybilError): b.validate_block(block) @@ -726,7 +763,7 @@ class TestMultipleInputs(object): tx = tx.sign([user_sk]) # validate transaction - assert b.is_valid_transaction(tx) == tx + tx.validate(b) assert len(tx.inputs) == 1 assert len(tx.outputs) == 1 @@ -748,7 +785,7 @@ class TestMultipleInputs(object): asset_id=input_tx.id) tx = tx.sign([user_sk]) - assert b.is_valid_transaction(tx) == tx + tx.validate(b) assert len(tx.inputs) == 1 assert len(tx.outputs) == 1 @@ -780,7 +817,7 @@ class TestMultipleInputs(object): transfer_tx = transfer_tx.sign([user_sk, user2_sk]) # validate transaction - assert b.is_valid_transaction(transfer_tx) == transfer_tx + transfer_tx.validate(b) assert len(transfer_tx.inputs) == 1 assert len(transfer_tx.outputs) == 1 @@ -813,7 +850,7 @@ class TestMultipleInputs(object): asset_id=tx_input.id) tx = tx.sign([user_sk, user2_sk]) - assert b.is_valid_transaction(tx) == tx + tx.validate(b) assert len(tx.inputs) == 1 assert len(tx.outputs) == 1 @@ -1167,7 +1204,6 @@ def test_cant_spend_same_input_twice_in_tx(b, genesis_block): tx_transfer = Transaction.transfer(dup_inputs, [([b.me], 200)], asset_id=tx_create.id) tx_transfer_signed = tx_transfer.sign([b.me_private]) - assert b.is_valid_transaction(tx_transfer_signed) is False with pytest.raises(DoubleSpend): tx_transfer_signed.validate(b) @@ -1225,3 +1261,10 @@ def test_is_new_transaction(b, genesis_block): # Tx is new because it's only found in an invalid block assert b.is_new_transaction(tx.id) assert b.is_new_transaction(tx.id, exclude_block_id=block.id) + + +def test_validate_asset_id_string(signed_transfer_tx): + from bigchaindb.common.exceptions import ValidationError + signed_transfer_tx.asset['id'] = 1 + with pytest.raises(ValidationError): + signed_transfer_tx.validate(None) diff --git a/tests/pipelines/test_block_creation.py b/tests/pipelines/test_block_creation.py index b7d3e03e..27efc65d 100644 --- a/tests/pipelines/test_block_creation.py +++ b/tests/pipelines/test_block_creation.py @@ -46,28 +46,19 @@ def test_validate_transaction_handles_exceptions(b, signed_create_tx): """ from bigchaindb.pipelines.block import BlockPipeline block_maker = BlockPipeline() + from bigchaindb.common.exceptions import ValidationError - # Test SchemaValidationError tx_dict = signed_create_tx.to_dict() - tx_dict['invalid_key'] = 'schema validation gonna getcha!' - assert block_maker.validate_tx(tx_dict) is None - # Test InvalidHash - tx_dict = signed_create_tx.to_dict() - tx_dict['id'] = 'a' * 64 - assert block_maker.validate_tx(tx_dict) is None + with patch('bigchaindb.models.Transaction.validate') as validate: + # Assert that validationerror gets caught + validate.side_effect = ValidationError() + assert block_maker.validate_tx(tx_dict) is None - # Test InvalidSignature when we pass a bad fulfillment - tx_dict = signed_create_tx.to_dict() - tx_dict['inputs'][0]['fulfillment'] = 'cf:0:aaaaaaaaaaaaaaaaaaaaaaaaa' - assert block_maker.validate_tx(tx_dict) is None - - # Test AmountError - signed_create_tx.outputs[0].amount = 0 - tx_dict = signed_create_tx.to_dict() - # set the correct value back so that we can continue using it - signed_create_tx.outputs[0].amount = 1 - assert block_maker.validate_tx(tx_dict) is None + # Assert that another error doesnt + validate.side_effect = IOError() + with pytest.raises(IOError): + block_maker.validate_tx(tx_dict) def test_create_block(b, user_pk): diff --git a/tests/pipelines/test_stale_monitor.py b/tests/pipelines/test_stale_monitor.py index 06ee5b5f..6e2b12b8 100644 --- a/tests/pipelines/test_stale_monitor.py +++ b/tests/pipelines/test_stale_monitor.py @@ -36,7 +36,11 @@ def test_reassign_transactions(b, user_pk): stm = stale.StaleTransactionMonitor(timeout=0.001, backlog_reassign_delay=0.001) - stm.reassign_transactions(tx.to_dict()) + # This worked previously because transaction['assignee'] was only used if + # bigchain.nodes_except_me was not empty. + tx_dict = tx.to_dict() + tx_dict['assignee'] = b.me + stm.reassign_transactions(tx_dict) # test with federation tx = Transaction.create([b.me], [([user_pk], 1)]) @@ -58,7 +62,7 @@ def test_reassign_transactions(b, user_pk): tx = tx.sign([b.me_private]) stm.bigchain.nodes_except_me = ['lol'] b.write_transaction(tx) - stm.bigchain.nodes_except_me = None + stm.bigchain.nodes_except_me = [] tx = list(query.get_stale_transactions(b.connection, 0))[0] stm.reassign_transactions(tx) diff --git a/tests/pipelines/test_vote.py b/tests/pipelines/test_vote.py index 20beac1e..fa167d17 100644 --- a/tests/pipelines/test_vote.py +++ b/tests/pipelines/test_vote.py @@ -128,17 +128,23 @@ def test_validate_block_with_invalid_signature(b): @pytest.mark.genesis def test_vote_validate_transaction(b): from bigchaindb.pipelines import vote - from bigchaindb.models import Transaction + from bigchaindb.common.exceptions import ValidationError tx = dummy_tx(b) vote_obj = vote.Vote() validation = vote_obj.validate_tx(tx, 123, 1) assert validation == (True, 123, 1) - # NOTE: Submit unsigned transaction to `validate_tx` yields `False`. - tx = Transaction.create([b.me], [([b.me], 1)]) - validation = vote_obj.validate_tx(tx, 456, 10) - assert validation == (False, 456, 10) + with patch('bigchaindb.models.Transaction.validate') as validate: + # Assert that validationerror gets caught + validate.side_effect = ValidationError() + validation = vote_obj.validate_tx(tx, 456, 10) + assert validation == (False, 456, 10) + + # Assert that another error doesnt + validate.side_effect = IOError() + with pytest.raises(IOError): + validation = vote_obj.validate_tx(tx, 456, 10) @pytest.mark.genesis diff --git a/tests/test_core.py b/tests/test_core.py index 8e0a63fc..f939ad05 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -80,13 +80,3 @@ def test_get_blocks_status_containing_tx(monkeypatch): bigchain = Bigchain(public_key='pubkey', private_key='privkey') with pytest.raises(Exception): bigchain.get_blocks_status_containing_tx('txid') - - -def test_has_previous_vote(monkeypatch): - from bigchaindb.core import Bigchain - monkeypatch.setattr( - 'bigchaindb.utils.verify_vote_signature', lambda voters, vote: False) - bigchain = Bigchain(public_key='pubkey', private_key='privkey') - block = {'votes': ({'node_pubkey': 'pubkey'},)} - with pytest.raises(Exception): - bigchain.has_previous_vote(block) diff --git a/tests/test_models.py b/tests/test_models.py index 975d9ea1..8acf6507 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -1,4 +1,5 @@ from pytest import raises +from bigchaindb.common.exceptions import ValidationError class TestTransactionModel(object): @@ -8,12 +9,12 @@ class TestTransactionModel(object): tx = Transaction.create([b.me], [([b.me], 1)]) tx.operation = 'something invalid' - with raises(TypeError): + with raises(ValidationError): tx.validate(b) tx.operation = 'CREATE' tx.inputs = [] - with raises(ValueError): + with raises(ValidationError): tx.validate(b) @@ -61,11 +62,10 @@ class TestBlockModel(object): assert block.to_dict() == expected def test_block_invalid_serializaton(self): - from bigchaindb.common.exceptions import OperationError from bigchaindb.models import Block block = Block([]) - with raises(OperationError): + with raises(ValueError): block.to_dict() def test_block_deserialization(self, b): diff --git a/tests/web/test_transactions.py b/tests/web/test_transactions.py index 71f4f0e9..5533dbd0 100644 --- a/tests/web/test_transactions.py +++ b/tests/web/test_transactions.py @@ -1,4 +1,3 @@ -import builtins import json from unittest.mock import patch @@ -113,18 +112,15 @@ def test_post_create_transaction_with_invalid_schema(client, caplog): ('DoubleSpend', 'Nope! It is gone now!'), ('InvalidHash', 'Do not smoke that!'), ('InvalidSignature', 'Falsche Unterschrift!'), - ('OperationError', 'Create and transfer!'), - ('TransactionDoesNotExist', 'Hallucinations?'), + ('ValidationError', 'Create and transfer!'), + ('InputDoesNotExist', 'Hallucinations?'), ('TransactionOwnerError', 'Not yours!'), ('TransactionNotInValidBlock', 'Wait, maybe?'), - ('ValueError', '?'), + ('ValidationError', '?'), )) def test_post_invalid_transaction(client, exc, msg, monkeypatch, caplog): from bigchaindb.common import exceptions - try: - exc_cls = getattr(exceptions, exc) - except AttributeError: - exc_cls = getattr(builtins, 'ValueError') + exc_cls = getattr(exceptions, exc) def mock_validation(self_, tx): raise exc_cls(msg) From 0fb4ea424b16c965f6f7579cfc1016df23fae28f Mon Sep 17 00:00:00 2001 From: Scott Sadler Date: Mon, 13 Mar 2017 14:55:03 +0100 Subject: [PATCH 21/23] remove stray validation --- bigchaindb/models.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/bigchaindb/models.py b/bigchaindb/models.py index bf6bafd5..771d6d6a 100644 --- a/bigchaindb/models.py +++ b/bigchaindb/models.py @@ -220,9 +220,6 @@ class Block(object): if self.node_pubkey not in bigchain.federation: raise SybilError('Only federation nodes can create blocks') - if set(self.voters) != bigchain.federation: - raise SybilError('Block voters differs from server keyring') - # Check that the signature is valid if not self.is_signature_valid(): raise InvalidSignature('Invalid block signature') From 58a1a25d43ab78d8b47a0b6567cc732f5ab89590 Mon Sep 17 00:00:00 2001 From: Scott Sadler Date: Mon, 13 Mar 2017 16:26:41 +0100 Subject: [PATCH 22/23] test for invalid vote in election pipeline --- bigchaindb/pipelines/election.py | 2 +- tests/pipelines/test_election.py | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/bigchaindb/pipelines/election.py b/bigchaindb/pipelines/election.py index fe4fbc68..a5818b3e 100644 --- a/bigchaindb/pipelines/election.py +++ b/bigchaindb/pipelines/election.py @@ -36,7 +36,7 @@ class Election: try: block_id = next_vote['vote']['voting_for_block'] node = next_vote['node_pubkey'] - except IndexError: + except KeyError: return next_block = self.bigchain.get_block(block_id) diff --git a/tests/pipelines/test_election.py b/tests/pipelines/test_election.py index e7491656..3127dcaf 100644 --- a/tests/pipelines/test_election.py +++ b/tests/pipelines/test_election.py @@ -114,6 +114,13 @@ def test_check_for_quorum_valid(b, user_pk): assert e.check_for_quorum(votes[-1]) is None +@patch('bigchaindb.core.Bigchain.get_block') +def test_invalid_vote(get_block, b): + e = election.Election() + assert e.check_for_quorum({}) is None + get_block.assert_not_called() + + @pytest.mark.bdb def test_check_requeue_transaction(b, user_pk): from bigchaindb.models import Transaction From e0366468ecb46181ecc4810dea3631f74000d271 Mon Sep 17 00:00:00 2001 From: Troy McConaghy Date: Mon, 13 Mar 2017 18:14:18 +0100 Subject: [PATCH 23/23] Fix comments in bigchaindb/toolbox Dockerfile The comments were referring to another Docker image (`krish7919/toolbox`). --- k8s/toolbox/Dockerfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/k8s/toolbox/Dockerfile b/k8s/toolbox/Dockerfile index 6bcb1298..bac50f0a 100644 --- a/k8s/toolbox/Dockerfile +++ b/k8s/toolbox/Dockerfile @@ -1,7 +1,7 @@ # Toolbox container for debugging # Run as: -# docker run -it --rm --entrypoint sh krish7919/toolbox -# kubectl run -it toolbox --image krish7919/toolbox --restart=Never --rm +# docker run -it --rm --entrypoint sh bigchaindb/toolbox +# kubectl run -it toolbox --image bigchaindb/toolbox --restart=Never --rm FROM alpine:3.5 MAINTAINER github.com/krish7919