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.

Sign up for GitHub

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

Explicit provider ETH addr instead of derived from ETH pub key #336

Merged
merged 14 commits into from
Jun 3, 2019
Merged

Explicit provider ETH addr instead of derived from ETH pub key #336

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
009a0d0
[WTCT] explicit ETH key instead of derivation from ETH pub key
tworec Apr 15, 2019
42c23ab
Merge branch 'num_cores_out' into provider_eth_addr
tworec Apr 15, 2019
dbf8606
Merge branch 'master' into provider_eth_addr
tworec May 9, 2019
b3c8937
[factories] two convenience random eth functions
tworec May 9, 2019
8d01a04
more general pubkey_to_address function
tworec May 9, 2019
aa78f3e
concent promissory note test fix : unbound provider id from provider …
tworec May 9, 2019
093920f
restore WTCT factory logic: pub_key bound to eth_addr
tworec May 10, 2019
8a70667
two more tests for better test coverage ;)
tworec May 10, 2019
d1070e9
test comment
tworec May 10, 2019
70e0416
__init__.py for test module + deprecation warn fix
tworec May 10, 2019
fb97112
type hint corrected (kudos to @krigpl)
tworec May 13, 2019
089f077
Merge branch 'master' into provider_eth_addr
tworec May 31, 2019
27add20
pylint: ignore imports when computing similarities
tworec May 31, 2019
1cec8a1
after revie: lazy public key in THF restored + imports reorder
tworec Jun 3, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions golem_messages/factories/concents.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,10 @@
import factory.fuzzy
import faker

from golem_messages import cryptography
from golem_messages.factories import tasks as tasks_factories
from golem_messages.factories.helpers import random_eth_pub_key
from golem_messages.message import concents
from golem_messages.message.concents import FileTransferToken
from golem_messages.utils import encode_hex as encode_key_id
from . import helpers
from .tasks import (
SubtaskResultsAcceptedFactory, SubtaskResultsRejectedFactory
Expand Down Expand Up @@ -273,8 +272,7 @@ class Meta:
model = concents.ForcePaymentCommitted

payment_ts = factory.LazyFunction(lambda: int(time.time()))
task_owner_key = factory.LazyFunction(
lambda: encode_key_id(cryptography.ECCx(None).raw_pubkey))
task_owner_key = factory.LazyFunction(lambda: random_eth_pub_key())
provider_eth_account = factory.LazyFunction(
lambda: '0x' + faker.Faker().sha1())
amount_paid = factory.LazyFunction(
Expand Down
6 changes: 2 additions & 4 deletions golem_messages/factories/datastructures/tasks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from golem_messages.datastructures import tasks as dt_tasks
from golem_messages.factories import helpers
from golem_messages.factories.datastructures import p2p as dt_p2p_factories
from golem_messages.factories.helpers import random_eth_pub_key
from golem_messages.utils import encode_hex as encode_key_id


Expand All @@ -16,10 +17,7 @@ class Meta:
model = dt_tasks.TaskHeader
exclude = ('requestor_public_key', )

requestor_public_key = factory.LazyFunction(
lambda: encode_key_id(cryptography.ECCx(None).raw_pubkey)
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should still be LazyFunction to get different pubkeys everytime.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


requestor_public_key = random_eth_pub_key()
mask = factory.Faker('binary', length=masking.Mask.MASK_BYTES)
timestamp = factory.LazyFunction(lambda: int(time.time()))
task_id = factory.LazyAttribute(
Expand Down
11 changes: 10 additions & 1 deletion golem_messages/factories/helpers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@
import factory
import faker

from golem_messages import datastructures
from golem_messages import datastructures, cryptography
from golem_messages import idgenerator
from golem_messages.utils import encode_hex, pubkey_to_address

if typing.TYPE_CHECKING:
from golem_messages.message.base import Message # noqa pylint:disable=unused-import
Expand Down Expand Up @@ -90,6 +91,14 @@ def fake_version() -> str:
)


def random_eth_pub_key():
return encode_hex(cryptography.ECCx(None).raw_pubkey)


def random_eth_address():
return pubkey_to_address(random_eth_pub_key())


class MessageFactory(factory.Factory):

@staticmethod
Expand Down
31 changes: 18 additions & 13 deletions golem_messages/factories/tasks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
# pylint: disable=too-few-public-methods,unnecessary-lambda
import calendar
from contextlib import suppress
import datetime
import time
import typing
from contextlib import suppress
tworec marked this conversation as resolved.
Show resolved Hide resolved

import factory.fuzzy
import faker

from golem_messages import cryptography
from golem_messages.factories.datastructures.tasks import TaskHeaderFactory
from golem_messages.utils import encode_hex as encode_key_id
from golem_messages.factories.helpers import random_eth_pub_key
from golem_messages.message import tasks

from golem_messages.utils import encode_hex, pubkey_to_address
from . import helpers


Expand All @@ -21,11 +21,17 @@ class Meta:
model = tasks.WantToComputeTask

node_name = factory.Faker('name')
provider_public_key = factory.LazyFunction(
lambda: encode_key_id(cryptography.ECCx(None).raw_pubkey))
provider_ethereum_public_key = factory.SelfAttribute(
'provider_public_key'
)
provider_public_key = factory.LazyFunction(lambda: random_eth_pub_key())
# provider_ethereum_address is not bound to provider_public_key
# below binding is only for compatibility with Concent tests
# it should be like this
# ```
# provider_ethereum_address = factory.LazyFunction(
# lambda: random_eth_address())
# ```
provider_ethereum_address = factory.LazyAttribute(
lambda o: pubkey_to_address(o.provider_public_key))

task_header = factory.SubFactory(TaskHeaderFactory)


Expand Down Expand Up @@ -68,8 +74,7 @@ class Meta:
lambda o: o.want_to_compute_task.provider_public_key
)
compute_task_def = factory.SubFactory(ComputeTaskDefFactory)
requestor_public_key = factory.LazyFunction(
lambda: encode_key_id(cryptography.ECCx(None).raw_pubkey))
requestor_public_key = factory.LazyFunction(lambda: random_eth_pub_key())
want_to_compute_task = factory.SubFactory(WantToComputeTaskFactory)
package_hash = factory.LazyFunction(lambda: 'sha1:' + faker.Faker().sha1())
size = factory.Faker('random_int', min=1 << 20, max=10 << 20)
Expand All @@ -92,7 +97,7 @@ def with_signed_nested_messages(
WTCT_TH_KEY = 'want_to_compute_task__task_header' # noqa
WTCT_KEY = 'want_to_compute_task' # noqa
if requestor_keys:
encoded_pubkey = encode_key_id(requestor_keys.raw_pubkey)
encoded_pubkey = encode_hex(requestor_keys.raw_pubkey)
# initialize the TTC's requestor public key from the requestor pair
if 'requestor_public_key' not in kwargs:
kwargs['requestor_public_key'] = encoded_pubkey
Expand Down Expand Up @@ -128,7 +133,7 @@ def with_signed_nested_messages(
WTCT_KEY + '__provider_public_key' not in kwargs
):
kwargs[WTCT_KEY + '__provider_public_key'] = \
encode_key_id(provider_keys.raw_pubkey)
encode_hex(provider_keys.raw_pubkey)

# initialize the WantToComputeTask's signature private key from
# the provider key pair
Expand Down Expand Up @@ -215,7 +220,7 @@ def ethsig(
if not privkey and not ttc.requestor_ethereum_public_key:
keys = keys or cryptography.ECCx(None)
privkey = keys.raw_privkey
ttc.requestor_ethereum_public_key = encode_key_id(keys.raw_pubkey)
ttc.requestor_ethereum_public_key = encode_hex(keys.raw_pubkey)

if privkey:
ttc.generate_ethsig(privkey)
Expand Down
10 changes: 1 addition & 9 deletions golem_messages/message/tasks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ class WantToComputeTask(ConcentEnabled, base.Message):
# contents of this field.

'provider_public_key', # for signing and encryption
'provider_ethereum_public_key', # for transactions on ETH blockchain
'provider_ethereum_address', # for transactions on ETH blockchain
'task_header', # Demand; signed by a Requestor
] + base.Message.__slots__

Expand All @@ -266,10 +266,6 @@ def __init__(self, *args, **kwargs):
if self.num_subtasks is None:
self.num_subtasks = self.DEFAULT_NUM_SUBTASKS

@property
def provider_ethereum_address(self):
return pubkey_to_address(self.provider_ethereum_public_key)

@property
def task_id(self):
return self.task_header.task_id
Expand Down Expand Up @@ -332,10 +328,6 @@ def requestor_ethereum_address(self):
def provider_public_key(self):
return self.want_to_compute_task.provider_public_key

@property
def provider_ethereum_public_key(self):
return self.want_to_compute_task.provider_ethereum_public_key

@property
def provider_ethereum_address(self):
return self.want_to_compute_task.provider_ethereum_address
Expand Down
7 changes: 4 additions & 3 deletions golem_messages/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,18 @@
import copy
from uuid import UUID

from eth_utils import to_checksum_address, keccak
from eth_utils import to_checksum_address, keccak, hexstr_if_str

from golem_messages import message


def pubkey_to_address(pubkey: str) -> str:
"""
convert a hex-encoded pubkey into a checksummed address
convert pubkey into a checksummed address
pubkey might be hex str or bytes or int
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we don't do that here. pubkey is a string, is typed as such and leave it this way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is that? can you provide some rationale?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

throw-anything-as-an-argument-and-hope-for-the-best just doesn't seem like a maintainable approach. Easier to write and read code when you know the type of the variable.
Also better integration with tools like static analyzer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how it works in eth lib used by us here. Its a standard there. We support three types of input, not 'anything': string, hexstr and int.

It is usable in such a form but I can introduce dedicated params...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But we don't need to support three types. At this point almost everywhere we're working on strings. So just provide strings and that's it.

Copy link
Contributor Author

@tworec tworec May 13, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I needed it to support bytes representation and it seems to me I've found a pretty concise and effective way to achieve it. Much better, I think, that you can observe 5 lines below -- just expand this view and you'll see, that next util function already supports different types on the single input parameter.

This is "The Python Way":

One thing you've noticed must be corrected indeed -- the parameter type.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Python way is also writing unmaintainable and illegible code, doesn't mean we should follow that.
This is the first Union type hint in this repository - meaning, what I said earlier, we don't do that here. We should be consistent within the same project and this case is way too minor to break current conventions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't approve this change but I have nothing else to add and am dismissing my review since this won't be resolved.

"""
return to_checksum_address(
keccak(decode_hex(pubkey))[12:].hex(),
hexstr_if_str(keccak, pubkey)[12:].hex(),
)


Expand Down
14 changes: 7 additions & 7 deletions tests/message/test_concents.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,19 @@
# pylint: disable=protected-access
from datetime import datetime, timedelta
import math
import time
import unittest
from datetime import datetime, timedelta
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now it's not in alphabetical order. I preferred previous ordering.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

made by pycharm import optimizator, reverting


import freezegun

from golem_messages import cryptography
from golem_messages.datastructures import promissory
from golem_messages import exceptions
from golem_messages import message

from golem_messages import factories
from golem_messages import message
from golem_messages import shortcuts
from golem_messages.datastructures import promissory
from golem_messages.message import concents
from golem_messages.utils import encode_hex as encode_key_id

from golem_messages.utils import pubkey_to_address
from tests.message import helpers
from tests.message import mixins

Expand Down Expand Up @@ -142,7 +140,9 @@ def test_subtask_id(self):
def test_concent_promissory_note(self):
provider_keys = cryptography.ECCx(None)
wtct = factories.tasks.WantToComputeTaskFactory(
provider_public_key=encode_key_id(provider_keys.raw_pubkey)
provider_ethereum_address=pubkey_to_address(
provider_keys.raw_pubkey
)
)
srv: concents.SubtaskResultsVerify = self.FACTORY(**{
'subtask_results_rejected__'
Expand Down
25 changes: 4 additions & 21 deletions tests/message/test_tasks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@
from golem_messages.datastructures import promissory
from golem_messages.datastructures.tasks import TaskHeader
from golem_messages.factories.datastructures.tasks import TaskHeaderFactory
from golem_messages.factories.helpers import override_timestamp
from golem_messages.factories.helpers import override_timestamp, \
random_eth_pub_key
from golem_messages.utils import encode_hex, decode_hex
from tests.message import mixins, helpers

Expand Down Expand Up @@ -48,17 +49,9 @@ def test_extra_data(self):

def test_provider_ethereum_address_checksum(self):
wtct = self.FACTORY()
self.assertTrue(wtct.provider_ethereum_public_key)
self.assertTrue(wtct.provider_ethereum_address)
self.assertTrue(is_checksum_address(wtct.provider_ethereum_address))

def test_ethereum_address_provider(self):
wtct = self.FACTORY()
provider_public_key = decode_hex(wtct.provider_ethereum_public_key)

self.assertEqual(wtct.provider_ethereum_address,
to_checksum_address(
'0x' + sha3(provider_public_key)[12:].hex()))

def test_ethereum_address(self):
wtct = self.FACTORY()
serialized = shortcuts.dump(wtct, None, None)
Expand Down Expand Up @@ -283,20 +276,11 @@ def test_ethereum_address_requestor(self):

def test_ethereum_address_provider(self):
msg = factories.tasks.TaskToComputeFactory()
provider_public_key = decode_hex(msg.provider_ethereum_public_key)
serialized = shortcuts.dump(msg, None, None)
msg_l = shortcuts.load(serialized, None, None)
self.assertEqual(len(msg_l.provider_ethereum_address), 2 + (20*2))
self.assertEqual(msg_l.provider_ethereum_address,
msg_l.want_to_compute_task.provider_ethereum_address)
self.assertEqual(msg.provider_ethereum_address,
to_checksum_address(
'0x' + sha3(provider_public_key)[12:].hex()))

def test_public_key_provider(self):
msg = factories.tasks.TaskToComputeFactory()
self.assertEqual(msg.provider_ethereum_public_key,
msg.want_to_compute_task.provider_ethereum_public_key)

def test_task_id(self):
self.assertEqual(self.msg.task_id,
Expand Down Expand Up @@ -517,8 +501,7 @@ def test_ethsig(self):
def test_ethsig_none(self):
msg: message.tasks.TaskToCompute = \
factories.tasks.TaskToComputeFactory(
requestor_ethereum_public_key=encode_hex(
cryptography.ECCx(None).raw_pubkey))
requestor_ethereum_public_key=random_eth_pub_key())
self.assertIsNone(msg.ethsig)
with self.assertRaises(exceptions.InvalidSignature):
helpers.dump_and_load(msg)
Expand Down
37 changes: 33 additions & 4 deletions tests/test_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from uuid import UUID
import unittest
from uuid import UUID

from golem_messages import factories
from golem_messages import utils
Expand All @@ -21,12 +21,41 @@ def test_copy_and_sign(self):
copied_msg.sig = None
self.assertEqual(copied_msg, msg)

def test_pubkey_to_address(self):
encoded_pubkey = 'd029f6c7fe83148774dec2f19cc095b3' \
def test_hex_pubkey_to_address(self):
hex_pubkey = 'd029f6c7fe83148774dec2f19cc095b3' \
'75a362e7684a5cdab7ac4587fc146a72' \
'6494f96ddc38635ceccdeba4ec14784d' \
'735511acccabc29effdf584a3589a86e'
self.assertEqual(
utils.pubkey_to_address(encoded_pubkey),
utils.pubkey_to_address(hex_pubkey),
'0xA2B1b6644F953d3d8488C35132045CC235A568E4'
)

def test_0x_hex_pubkey_to_address(self):
hex_pubkey = '0xd029f6c7fe83148774dec2f19cc095b3' \
'75a362e7684a5cdab7ac4587fc146a72' \
'6494f96ddc38635ceccdeba4ec14784d' \
'735511acccabc29effdf584a3589a86e'
self.assertEqual(
utils.pubkey_to_address(hex_pubkey),
'0xA2B1b6644F953d3d8488C35132045CC235A568E4'
)

def test_bytes_pubkey_to_address(self):
bytes_pubkey = b"1\xd1\x11\x02\xaeM\x84\xb0#\x02\xcd(\xddg\xc2e" \
b"\x90\xac\x93\x83*;/\x16),\xa2\xdb\xc2\xad\x99G" \
b"\xac~\x88t\x8dwQ\x0c\xb1`\x8d\xc7\x8f.\xe4MPT" \
b"\xea\xc7\xedb\xac'\xf3\xe7\x00\xf1\x1d\xf0\xe4\xfb"
self.assertEqual(
utils.pubkey_to_address(bytes_pubkey),
'0xF74a023B90AA8Af867E8ad56afeBfF66E2fDBd46'
)

def test_int_pubkey_to_address(self):
int_pubkey = int('2609110495624912638150583483236841806553203533339234'
'9069053650983433801754752954860010295385868748581009'
'61903958556513559452241970505204661562102403622139')
self.assertEqual(
utils.pubkey_to_address(int_pubkey),
'0xF74a023B90AA8Af867E8ad56afeBfF66E2fDBd46'
)