ON HOLD: Double spending prevention using Secp256k1 keys on TrustChain #68

xoriole · 2018-03-05T15:12:52Z

Double spending prevention using Secp256k1 signatures on transactions.

tribler-ci · 2018-03-05T15:12:54Z

Can one of the admins verify this patch?

qstokkink

It doesn't pass the tests 🙍

qstokkink · 2018-03-07T08:31:28Z

retest this please

qstokkink · 2018-03-07T09:08:52Z

retest this please

qstokkink · 2018-03-07T09:11:40Z

ipv8/attestation/trustchain/block.py

@@ -273,6 +290,14 @@ def err(reason):
                linklinked = database.get_linked(link)
                if linklinked is not None and linklinked.hash != self.hash:
                    err("Double countersign fraud")
+                    if 'double_sig' in self.transaction and self.transaction['double_sig'] \
+                        and 'double_sig' in linklinked.transaction and linklinked.transactino['double_sig']:


transactino?

qstokkink · 2018-03-07T09:19:35Z

ipv8/keyvault/doublesign.py

+        return self.point(x, y)
+
+    def sub(self, lhs, rhs):
+        return lhs + -rhs


+ - is redundant

This is the way it should be. Point negation and addition.

qstokkink · 2018-03-07T09:23:10Z

test/util.py

@@ -166,4 +166,4 @@ def twisted_wrapper(arg):
    """
    if isinstance(arg, (int, long)):
        return lambda x: deferred(arg)(inlineCallbacks(x))
-    return deferred(timeout=1)(inlineCallbacks(arg))
+    return deferred(timeout=10)(inlineCallbacks(arg))


Please change this per test, not globally.

xoriole · 2018-03-07T11:14:37Z

retest this please

xoriole · 2018-03-07T12:59:00Z

retest this please

qstokkink · 2018-03-07T13:05:36Z

test/keyvault/test_doublesign.py needs to be added to https://github.com/Tribler/py-ipv8/blob/master/test_classes_list.txt

Updated test timeouts to fit for signatures

Changed test timeout Using gmpy2 integer for field operations

qstokkink · 2018-03-07T13:15:33Z

test/base.py

@@ -95,7 +95,7 @@ def add_node_to_experiment(self, node):
        self.nodes.append(node)

    @inlineCallbacks
-    def deliver_messages(self, timeout=.1):
+    def deliver_messages(self, timeout=10):


Please don't change the base timeouts.

qstokkink · 2018-03-07T13:20:32Z

retest this please

devos50 · 2018-03-12T14:49:59Z

retest this please

devos50 · 2018-03-12T14:57:49Z

retest this please

devos50 · 2018-03-12T14:59:32Z

retest this please

devos50 · 2018-03-12T15:01:27Z

retest this please

devos50 · 2018-03-12T15:03:01Z

retest this please

devos50 · 2018-03-12T15:04:29Z

retest this please

devos50 · 2018-03-12T15:06:24Z

retest this please

devos50 · 2018-03-12T15:07:35Z

retest this please

devos50

It looks ok but I have some minor comments.

In general, I would like to know more about the performance. Can we get some numbers to quantify the overhead of the double spend detection (not for this PR)?

devos50 · 2018-03-12T16:19:43Z

ipv8/attestation/trustchain/block.py

@@ -2,6 +2,7 @@

 import time

+from ipv8.keyvault import doublesign


Shouldn't we use relative imports everywhere?

devos50 · 2018-03-12T16:20:13Z

ipv8/attestation/trustchain/block.py

@@ -117,8 +118,9 @@ def pack(self, signature=True):
        :param signature: False to pack EMPTY_SIG in the signature location, true to pack the signature field
        :return: the buffer the data was packed into
        """
+        raw_transaction = {i:self.transaction[i] for i in self.transaction if i != 'double_sig'}


Please add a space after the first : for clarity.

devos50 · 2018-03-12T16:21:23Z

ipv8/attestation/trustchain/block.py

@@ -292,15 +317,22 @@ def err(reason):
                err("Next hash is not equal to the hash id of the block")
                # Again, this might not be fraud, but fixing it can only result in fraud.

-        return result[0], errors
+        return result[0], errors, result[1]


We're already returning a tuple with three results now. Maybe we should convert to a namedtuple?

devos50 · 2018-03-12T16:21:42Z

ipv8/attestation/trustchain/block.py


-    def sign(self, key):
+    def sign(self, key, double_sign=False):


Please add a docstring about the double_sign parameter 👍

devos50 · 2018-03-13T11:28:24Z

ipv8/attestation/trustchain/block.py

+    @staticmethod
+    def double_spend():
+        """
+        The block violates at least one validation rule


Does the docstring match with the method name?

devos50 · 2018-03-13T11:29:47Z

ipv8/attestation/trustchain/community.py

@@ -51,11 +51,13 @@ class TrustChainCommunity(Community):
    def __init__(self, *args, **kwargs):
        working_directory = kwargs.pop('working_directory', '')
        db_name = kwargs.pop('db_name', self.DB_NAME)
+        double_sign = kwargs.pop('double_sign', False)


Please rename this to check_for_double_sign or something similar 👍

devos50 · 2018-03-13T11:31:47Z

ipv8/keyvault/crypto.py

+
+    def is_valid_double_signature(self, data, signature):
+        """
+        Returns True if signature is valid


Minor detail: please be consistent with ending docstrings with a period or not :)

qstokkink · 2018-10-23T12:00:47Z

retest this please

qstokkink · 2018-10-23T12:17:38Z

Merged into the nuclear_backdoor branch.

xoriole changed the title ~~Double spending prevention using Secp256k1 keys on TrustChain~~ WIP: Double spending prevention using Secp256k1 keys on TrustChain Mar 5, 2018

xoriole requested a review from qstokkink March 5, 2018 15:15

qstokkink requested changes Mar 5, 2018

View reviewed changes

xoriole force-pushed the double_spending_prevention branch 8 times, most recently from b01940f to b30809a Compare March 6, 2018 09:40

qstokkink reviewed Mar 7, 2018

View reviewed changes

xoriole force-pushed the double_spending_prevention branch from 9818aeb to 265fc07 Compare March 7, 2018 09:19

qstokkink reviewed Mar 7, 2018

View reviewed changes

xoriole force-pushed the double_spending_prevention branch 9 times, most recently from 1d6dc0d to 3e3f2e5 Compare March 7, 2018 11:13

xoriole force-pushed the double_spending_prevention branch 2 times, most recently from 52790c6 to f9fbb01 Compare March 7, 2018 12:56

xoriole added 2 commits March 7, 2018 14:08

Double spending detection using Secp256k1 keys

debbbad

Updated test timeouts to fit for signatures

Using signer secret key instead of crypto secret key

b06c70b

Changed test timeout Using gmpy2 integer for field operations

xoriole force-pushed the double_spending_prevention branch from 1367765 to 040ced3 Compare March 7, 2018 13:09

qstokkink reviewed Mar 7, 2018

View reviewed changes

Added double_sign tests to list of test classes

05f9674

xoriole force-pushed the double_spending_prevention branch from 040ced3 to 05f9674 Compare March 7, 2018 13:18

qstokkink approved these changes Mar 7, 2018

View reviewed changes

xoriole changed the title ~~WIP: Double spending prevention using Secp256k1 keys on TrustChain~~ READY: Double spending prevention using Secp256k1 keys on TrustChain Mar 7, 2018

qstokkink requested a review from devos50 March 9, 2018 15:24

devos50 suggested changes Mar 13, 2018

View reviewed changes

devos50 mentioned this pull request Mar 16, 2018

A trustful blockchain-based token economy to prevent bandwidth free-riding Tribler/tribler#3337

Closed

9 tasks

qstokkink changed the title ~~READY: Double spending prevention using Secp256k1 keys on TrustChain~~ ON HOLD: Double spending prevention using Secp256k1 keys on TrustChain Apr 19, 2018

qstokkink added the on hold label Oct 23, 2018

qstokkink closed this Oct 23, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ON HOLD: Double spending prevention using Secp256k1 keys on TrustChain #68

ON HOLD: Double spending prevention using Secp256k1 keys on TrustChain #68

xoriole commented Mar 5, 2018

tribler-ci commented Mar 5, 2018

qstokkink left a comment

qstokkink commented Mar 7, 2018

qstokkink commented Mar 7, 2018

qstokkink Mar 7, 2018

qstokkink Mar 7, 2018

xoriole Mar 7, 2018

qstokkink Mar 7, 2018

xoriole commented Mar 7, 2018

xoriole commented Mar 7, 2018

qstokkink commented Mar 7, 2018

qstokkink Mar 7, 2018

qstokkink commented Mar 7, 2018

devos50 commented Mar 12, 2018

devos50 commented Mar 12, 2018

devos50 commented Mar 12, 2018

devos50 commented Mar 12, 2018

devos50 commented Mar 12, 2018

devos50 commented Mar 12, 2018

devos50 commented Mar 12, 2018

devos50 commented Mar 12, 2018

devos50 left a comment

devos50 Mar 12, 2018

devos50 Mar 12, 2018

devos50 Mar 12, 2018

devos50 Mar 12, 2018

devos50 Mar 13, 2018

devos50 Mar 13, 2018

devos50 Mar 13, 2018

qstokkink commented Oct 23, 2018

qstokkink commented Oct 23, 2018

		@@ -2,6 +2,7 @@

		import time

		from ipv8.keyvault import doublesign

ON HOLD: Double spending prevention using Secp256k1 keys on TrustChain #68

ON HOLD: Double spending prevention using Secp256k1 keys on TrustChain #68

Conversation

xoriole commented Mar 5, 2018

tribler-ci commented Mar 5, 2018

qstokkink left a comment

Choose a reason for hiding this comment

qstokkink commented Mar 7, 2018

qstokkink commented Mar 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xoriole commented Mar 7, 2018

xoriole commented Mar 7, 2018

qstokkink commented Mar 7, 2018

Choose a reason for hiding this comment

qstokkink commented Mar 7, 2018

devos50 commented Mar 12, 2018

devos50 commented Mar 12, 2018

devos50 commented Mar 12, 2018

devos50 commented Mar 12, 2018

devos50 commented Mar 12, 2018

devos50 commented Mar 12, 2018

devos50 commented Mar 12, 2018

devos50 commented Mar 12, 2018

devos50 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qstokkink commented Oct 23, 2018

qstokkink commented Oct 23, 2018