From dbfcce34e73e71b9ea5a7c417c23377ac6ef24d1 Mon Sep 17 00:00:00 2001 From: Scott Sadler Date: Thu, 23 Feb 2017 12:07:38 +0100 Subject: [PATCH 01/21] 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/21] 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/21] 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/21] 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/21] 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/21] 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/21] 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/21] 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/21] 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/21] 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/21] 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/21] 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/21] 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/21] 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/21] 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/21] 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/21] 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 e011f50bc70318a19bd8abaa01c21c4e1b578baa Mon Sep 17 00:00:00 2001 From: Scott Sadler Date: Wed, 8 Mar 2017 17:33:35 +0100 Subject: [PATCH 18/21] 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 19/21] 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 20/21] 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 21/21] 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