Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistency between vote and transaction signing #238

Closed
rhsimplex opened this issue May 2, 2016 · 2 comments
Closed

Inconsistency between vote and transaction signing #238

rhsimplex opened this issue May 2, 2016 · 2 comments
Assignees

Comments

@rhsimplex
Copy link
Contributor

rhsimplex commented May 2, 2016

Logically, all consensus functionality is relegated to the consensus plugin. However vote signing is currently handled in core.py. Consider how a tx is signed -- a derived class function of AbstractConsensusRules in consensus.py called sign_transaction calls a function in util.py called sign_tx (phew!):

def sign_tx(transaction, private_key):
    """Sign a transaction

    A transaction signed with the `current_owner` corresponding private key.

    Args:
        transaction (dict): transaction to sign.
        private_key (str): base58 encoded private key to create a signature of the transaction.

    Returns:
        dict: transaction with the `signature` field included.

    """
    private_key = crypto.SigningKey(private_key)
    signature = private_key.sign(serialize(transaction))
    signed_transaction = transaction.copy()
    signed_transaction.update({'signature': signature.decode()})
    return signed_transaction

Contrast to this similar-looking function for casting a vote which is handled entirely in core.py:

    def vote(self, block, previous_block_id, decision, invalid_reason=None):
        """Cast your vote on the block given the previous_block_hash and the decision (valid/invalid)
        return the block to the updated in the database.

        Args:
            block (dict): Block to vote.
            previous_block_id (str): The id of the previous block.
            decision (bool): Whether the block is valid or invalid.
            invalid_reason (Optional[str]): Reason the block is invalid
        """

        vote = {
            'voting_for_block': block['id'],
            'previous_block': previous_block_id,
            'is_block_valid': decision,
            'invalid_reason': invalid_reason,
            'timestamp': util.timestamp()
        }

        vote_data = util.serialize(vote)
        signature = crypto.SigningKey(self.me_private).sign(vote_data).decode()

        vote_signed = {
            'node_pubkey': self.me,
            'signature': signature,
            'vote': vote
        }

        return vote_signed

The vote signing logic should also be handled by the consensus plugin. Votes, signatures, etc should all be signed in the same place to avoid duplicating code.

@r-marques
Copy link
Contributor

I agree with this. Lets take this into account when doing the refactor #312

@rhsimplex
Copy link
Contributor Author

Should be made irrelevant by #500

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants