From c1e3e4480bcf3fc82d896672bd0618ff77db45e6 Mon Sep 17 00:00:00 2001 From: Dariusz Rybi Date: Mon, 24 Jun 2019 11:51:23 +0200 Subject: [PATCH 01/10] Persistent topups and withdraws --- .gitignore | 1 + golem/ethereum/incomeskeeper.py | 38 +++++++++ golem/ethereum/paymentskeeper.py | 44 +++++++++- golem/ethereum/transactionsystem.py | 83 ++++++++++++++++++- tests/golem/ethereum/test_incomeskeeper.py | 24 ++++++ tests/golem/ethereum/test_paymentskeeper.py | 24 ++++++ .../golem/ethereum/test_transactionsystem.py | 7 +- 7 files changed, 210 insertions(+), 11 deletions(-) diff --git a/.gitignore b/.gitignore index 6665f7738e..435df008f0 100644 --- a/.gitignore +++ b/.gitignore @@ -66,6 +66,7 @@ yacctab.py data/ # Install and build +apps/rendering/resources/taskcollector/Release/* apps/rendering/resources/taskcollector/x64/Release/* apps/rendering/resources/taskcollector/Release/* apps/rendering/resources/taskcollector/taskcollector.vcxproj diff --git a/golem/ethereum/incomeskeeper.py b/golem/ethereum/incomeskeeper.py index cfa160d603..b06668ae18 100644 --- a/golem/ethereum/incomeskeeper.py +++ b/golem/ethereum/incomeskeeper.py @@ -15,6 +15,44 @@ class IncomesKeeper: """Keeps information about payments received from other nodes """ + @staticmethod + def received_eth_transfer( + tx_hash: str, + sender_address: str, + recipient_address: str, + amount: int, + ): + model.WalletOperation.create( + tx_hash=tx_hash, + direction=model.WalletOperation.DIRECTION.incoming, + operation_type=model.WalletOperation.TYPE.transfer, + status=model.WalletOperation.STATUS.confirmed, + sender_address=sender_address, + recipient_address=recipient_address, + amount=amount, + currency=model.WalletOperation.CURRENCY.ETH, + gas_cost=0, + ) + + @staticmethod + def received_gnt_transfer( + tx_hash: str, + sender_address: str, + recipient_address: str, + amount: int, + ): + model.WalletOperation.create( + tx_hash=tx_hash, + direction=model.WalletOperation.DIRECTION.incoming, + operation_type=model.WalletOperation.TYPE.transfer, + status=model.WalletOperation.STATUS.confirmed, + sender_address=sender_address, + recipient_address=recipient_address, + amount=amount, + currency=model.WalletOperation.CURRENCY.GNT, + gas_cost=0, + ) + @staticmethod def received_batch_transfer( tx_hash: str, diff --git a/golem/ethereum/paymentskeeper.py b/golem/ethereum/paymentskeeper.py index 50a145fa1e..8aade419cc 100644 --- a/golem/ethereum/paymentskeeper.py +++ b/golem/ethereum/paymentskeeper.py @@ -9,12 +9,15 @@ class PaymentsDatabase(object): - """ Save and retrieve from database information about payments that this node has to make / made + """Save and retrieve from database information + about payments that this node has to make / made """ @staticmethod def get_payment_value(subtask_id: str): - """ Return value of a payment that was done to the same node and for the same task as payment for payment_info + """Returns value of a payment + that was done to the same node and for the same + task as payment for payment_info """ return PaymentsDatabase.get_payment_for_subtask(subtask_id) @@ -64,12 +67,47 @@ def get_newest_payment(num: Optional[int] = None, class PaymentsKeeper: - """ Keeps information about payments for tasks that should be processed and send or received. """ + """Keeps information about payments for tasks + that should be processed and send or received. + """ def __init__(self) -> None: """ Create new payments keeper instance""" self.db = PaymentsDatabase() + @staticmethod + def sent_transfer( + tx_hash: str, + sender_address: str, + recipient_address: str, + amount: int, + currency: model.WalletOperation.CURRENCY, + ): + try: + operation = model.WalletOperation.select() \ + .where( + model.WalletOperation.tx_hash == tx_hash, + model.WalletOperation.operation_type # noqa + == model.WalletOperation.TYPE.transfer, + model.WalletOperation.direction # noqa + == model.WalletOperation.DIRECTION.outgoing, + model.WalletOperation.currency == currency + ).get() + operation.status = model.WalletOperation.STATUS.confirmed + operation.save() + except model.WalletOperation.DoesNotExist: + model.WalletOperation.create( + tx_hash=tx_hash, + direction=model.WalletOperation.DIRECTION.outgoing, + operation_type=model.WalletOperation.TYPE.transfer, + status=model.WalletOperation.STATUS.confirmed, + sender_address=sender_address, + recipient_address=recipient_address, + amount=amount, + currency=currency, + gas_cost=0, + ) + def get_list_of_all_payments(self, num: Optional[int] = None, interval: Optional[datetime.timedelta] = None): # This data is used by UI. diff --git a/golem/ethereum/transactionsystem.py b/golem/ethereum/transactionsystem.py index d727f72cd9..b6a26649ca 100644 --- a/golem/ethereum/transactionsystem.py +++ b/golem/ethereum/transactionsystem.py @@ -287,6 +287,46 @@ def _subscribe_to_events(self) -> None: ) ) + self._sci.subscribe_to_direct_incoming_eth_transfers( + address=self._sci.get_eth_address(), + from_block=from_block, + cb=lambda event: ik.received_eth_transfer( + tx_hash=event.tx_hash, + sender_address=event.from_address, + recipient_address=event.to_address, + amount=event.amount, + ), + ) + + self._sci.subscribe_to_gnt_transfers( + from_address=None, + to_address=self._sci.get_eth_address(), + from_blokc=from_block, + cb=lambda event: ik.received_gnt_transfer( + tx_hash=event.tx_hash, + sender_address=event.from_address, + recipient_address=event.to_address, + amount=event.amount, + ), + ) + # Overcome mypy limitations + gnt: model.WalletOperation.CURRENCY + gnt = model.WalletOperation.CURRENCY.GNT # type: ignore + # EO mypy limitations + + self._sci.subscribe_to_gnt_transfers( + from_address=self._sci.get_eth_address(), + to_address=None, + from_blokc=from_block, + cb=lambda event: self._payments_keeper.sent_transfer( + tx_hash=event.tx_hash, + sender_address=event.from_address, + recipient_address=event.to_address, + amount=event.amount, + currency=gnt, + ), + ) + if self.deposit_contract_available: self._sci.subscribe_to_forced_subtask_payments( None, @@ -589,12 +629,36 @@ def withdraw( available=self.get_available_eth(), currency=currency, ) - # TODO Create WalletOperation #4172 - return self._sci.transfer_eth( + tx_hash = self._sci.transfer_eth( destination, amount - gas_eth, gas_price, ) + model.WalletOperation.create( + tx_hash=tx_hash, + direction=model.WalletOperation.DIRECTION.outgoing, + operation_type=model.WalletOperation.TYPE.transfer, + status=model.WalletOperation.STATUS.sent, + sender_address=self._sci.get_eth_address(), + recipient_address=destination, + amount=amount, + currency=model.WalletOperation.CURRENCY.ETH, + gas_cost=gas_eth, + ) + + def on_eth_receipt(receipt): + if not receipt.status: + log.error("Failed ETH withdrawal: %r", receipt) + return + self._payments_keeper.sent_transfer( + tx_hash=receipt.tx_hash, + sender_address=receipt.from_address, + recipient_address=receipt.to_address, + amount=receipt.amount, + currency=model.WalletOperation.CURRENCY.ETH + ) + self._sci.on_transaction_confirmed(tx_hash, on_eth_receipt) + return tx_hash if currency == 'GNT': if amount > self.get_available_gnt(): @@ -603,18 +667,29 @@ def withdraw( available=self.get_available_gnt(), currency=currency, ) - # TODO Create WalletOperation #4172 tx_hash = self._sci.convert_gntb_to_gnt( destination, amount, gas_price, ) + if gas_price is None: + gas_price = self.gas_price + model.WalletOperation.create( + tx_hash=tx_hash, + direction=model.WalletOperation.DIRECTION.outgoing, + operation_type=model.WalletOperation.TYPE.transfer, + status=model.WalletOperation.STATUS.sent, + sender_address=self._sci.get_eth_address(), + recipient_address=destination, + amount=amount, + currency=model.WalletOperation.CURRENCY.GNT, + gas_cost=gas_price * self._sci.GAS_GNT_TRANSFER, + ) def on_receipt(receipt) -> None: self._gntb_withdrawn -= amount if not receipt.status: log.error("Failed GNTB withdrawal: %r", receipt) - # TODO Update WalletOperation #4172 self._sci.on_transaction_confirmed(tx_hash, on_receipt) self._gntb_withdrawn += amount return tx_hash diff --git a/tests/golem/ethereum/test_incomeskeeper.py b/tests/golem/ethereum/test_incomeskeeper.py index 3d1077596b..803b63f55c 100644 --- a/tests/golem/ethereum/test_incomeskeeper.py +++ b/tests/golem/ethereum/test_incomeskeeper.py @@ -289,3 +289,27 @@ def test_update_overdue_incomes_already_marked_as_overdue(self): income.wallet_operation.refresh().status, model.WalletOperation.STATUS.overdue, ) + + def test_received_eth_transfer(self): + self.incomes_keeper.received_eth_transfer( + tx_hash=f"0x{'0'*64}", + sender_address=random_eth_address(), + recipient_address=random_eth_address(), + amount=1, + ) + self.assertEqual( + model.WalletOperation.select().count(), + 1, + ) + + def test_received_gnt_transfer(self): + self.incomes_keeper.received_gnt_transfer( + tx_hash=f"0x{'0'*64}", + sender_address=random_eth_address(), + recipient_address=random_eth_address(), + amount=1, + ) + self.assertEqual( + model.WalletOperation.select().count(), + 1, + ) diff --git a/tests/golem/ethereum/test_paymentskeeper.py b/tests/golem/ethereum/test_paymentskeeper.py index ab5dff0a29..f8cd94f68a 100644 --- a/tests/golem/ethereum/test_paymentskeeper.py +++ b/tests/golem/ethereum/test_paymentskeeper.py @@ -1,5 +1,10 @@ +from golem_messages.factories.helpers import ( + random_eth_address, +) + from golem import model from golem.ethereum.paymentskeeper import PaymentsDatabase +from golem.ethereum.paymentskeeper import PaymentsKeeper from golem.tools.testwithdatabase import TestWithDatabase from tests.factories.model import TaskPayment as TaskPaymentFactory @@ -42,3 +47,22 @@ def test_subtasks_payments(self): payments = pd.get_subtasks_payments(['id1', 'id4', 'id2']) assert self._get_ids(payments) == ['id1', 'id2'] + + +class TestPaymentsKeeper(TestWithDatabase): + def setUp(self): + super().setUp() + self.payments_keeper = PaymentsKeeper() + + def test_sent_transfer(self): + self.payments_keeper.sent_transfer( + tx_hash=f"0x{'0'*64}", + sender_address=random_eth_address(), + recipient_address=random_eth_address(), + amount=1, + currency=model.WalletOperation.CURRENCY.GNT, + ) + self.assertEqual( + model.WalletOperation.select().count(), + 1, + ) diff --git a/tests/golem/ethereum/test_transactionsystem.py b/tests/golem/ethereum/test_transactionsystem.py index b9319e5437..8a9f14f8c3 100644 --- a/tests/golem/ethereum/test_transactionsystem.py +++ b/tests/golem/ethereum/test_transactionsystem.py @@ -40,6 +40,7 @@ def setUp(self): self.sci.get_gntb_balance.return_value = 0 self.sci.GAS_PER_PAYMENT = 20000 self.sci.get_deposit_locked_until.return_value = 0 + self.sci.GAS_GNT_TRANSFER = 2 self.ets = self._make_ets() def _make_ets( @@ -209,7 +210,6 @@ def test_convert_gnt(self): self.sci.get_eth_balance.return_value = denoms.ether self.sci.get_current_gas_price.return_value = 0 self.sci.GAS_OPEN_GATE = 10 - self.sci.GAS_GNT_TRANSFER = 2 self.sci.GAS_TRANSFER_FROM_GATE = 5 self.ets._refresh_balances() @@ -240,7 +240,6 @@ def test_topup_while_convert(self): self.sci.get_gnt_balance.return_value = amount1 self.sci.get_eth_balance.return_value = denoms.ether self.sci.get_current_gas_price.return_value = 0 - self.sci.GAS_GNT_TRANSFER = 2 self.sci.GAS_TRANSFER_FROM_GATE = 5 self.ets._refresh_balances() @@ -356,8 +355,8 @@ def setUp(self): self.sci.estimate_transfer_eth_gas.return_value = self.gas_cost self.dest = '0x' + 40 * 'd' - self.eth_tx = '0xee' - self.gntb_tx = '0xfad' + self.eth_tx = f'0x{"e"*64}' + self.gntb_tx = f'0x{"f"*64}' self.sci.transfer_eth.return_value = self.eth_tx self.sci.convert_gntb_to_gnt.return_value = self.gntb_tx From 8561d99b36c4eb21108f2ff50af94510007066b6 Mon Sep 17 00:00:00 2001 From: Dariusz Rybi Date: Tue, 25 Jun 2019 12:01:01 +0200 Subject: [PATCH 02/10] Move gas_price out of 'if' blocks --- .gitignore | 1 - golem/ethereum/transactionsystem.py | 10 +++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/.gitignore b/.gitignore index 435df008f0..6665f7738e 100644 --- a/.gitignore +++ b/.gitignore @@ -66,7 +66,6 @@ yacctab.py data/ # Install and build -apps/rendering/resources/taskcollector/Release/* apps/rendering/resources/taskcollector/x64/Release/* apps/rendering/resources/taskcollector/Release/* apps/rendering/resources/taskcollector/taskcollector.vcxproj diff --git a/golem/ethereum/transactionsystem.py b/golem/ethereum/transactionsystem.py index b6a26649ca..7e106ee525 100644 --- a/golem/ethereum/transactionsystem.py +++ b/golem/ethereum/transactionsystem.py @@ -317,7 +317,7 @@ def _subscribe_to_events(self) -> None: self._sci.subscribe_to_gnt_transfers( from_address=self._sci.get_eth_address(), to_address=None, - from_blokc=from_block, + from_block=from_block, cb=lambda event: self._payments_keeper.sent_transfer( tx_hash=event.tx_hash, sender_address=event.from_address, @@ -618,9 +618,11 @@ def withdraw( currency, destination, ) + + if gas_price is None: + gas_price = self.gas_price + if currency == 'ETH': - if gas_price is None: - gas_price = self.gas_price gas_eth = self.get_withdraw_gas_cost(amount, destination, currency)\ * gas_price if amount > self.get_available_eth(): @@ -672,8 +674,6 @@ def on_eth_receipt(receipt): amount, gas_price, ) - if gas_price is None: - gas_price = self.gas_price model.WalletOperation.create( tx_hash=tx_hash, direction=model.WalletOperation.DIRECTION.outgoing, From ef8eb05ae193d871e04efb8620b5448734880a03 Mon Sep 17 00:00:00 2001 From: Dariusz Rybi Date: Tue, 25 Jun 2019 12:22:37 +0200 Subject: [PATCH 03/10] [review] Simplify WHERE clause --- golem/ethereum/paymentskeeper.py | 26 ++++---------------------- golem/ethereum/transactionsystem.py | 12 ------------ 2 files changed, 4 insertions(+), 34 deletions(-) diff --git a/golem/ethereum/paymentskeeper.py b/golem/ethereum/paymentskeeper.py index 8aade419cc..0ac69ce52b 100644 --- a/golem/ethereum/paymentskeeper.py +++ b/golem/ethereum/paymentskeeper.py @@ -76,36 +76,18 @@ def __init__(self) -> None: self.db = PaymentsDatabase() @staticmethod - def sent_transfer( - tx_hash: str, - sender_address: str, - recipient_address: str, - amount: int, - currency: model.WalletOperation.CURRENCY, - ): + def sent_transfer(tx_hash: str): try: operation = model.WalletOperation.select() \ .where( model.WalletOperation.tx_hash == tx_hash, - model.WalletOperation.operation_type # noqa - == model.WalletOperation.TYPE.transfer, - model.WalletOperation.direction # noqa - == model.WalletOperation.DIRECTION.outgoing, - model.WalletOperation.currency == currency ).get() operation.status = model.WalletOperation.STATUS.confirmed operation.save() except model.WalletOperation.DoesNotExist: - model.WalletOperation.create( - tx_hash=tx_hash, - direction=model.WalletOperation.DIRECTION.outgoing, - operation_type=model.WalletOperation.TYPE.transfer, - status=model.WalletOperation.STATUS.confirmed, - sender_address=sender_address, - recipient_address=recipient_address, - amount=amount, - currency=currency, - gas_cost=0, + logger.warning( + "Got confirmation of unknown transfer. tx_hash=%s", + tx_hash, ) def get_list_of_all_payments(self, num: Optional[int] = None, diff --git a/golem/ethereum/transactionsystem.py b/golem/ethereum/transactionsystem.py index 7e106ee525..244e576737 100644 --- a/golem/ethereum/transactionsystem.py +++ b/golem/ethereum/transactionsystem.py @@ -309,10 +309,6 @@ def _subscribe_to_events(self) -> None: amount=event.amount, ), ) - # Overcome mypy limitations - gnt: model.WalletOperation.CURRENCY - gnt = model.WalletOperation.CURRENCY.GNT # type: ignore - # EO mypy limitations self._sci.subscribe_to_gnt_transfers( from_address=self._sci.get_eth_address(), @@ -320,10 +316,6 @@ def _subscribe_to_events(self) -> None: from_block=from_block, cb=lambda event: self._payments_keeper.sent_transfer( tx_hash=event.tx_hash, - sender_address=event.from_address, - recipient_address=event.to_address, - amount=event.amount, - currency=gnt, ), ) @@ -654,10 +646,6 @@ def on_eth_receipt(receipt): return self._payments_keeper.sent_transfer( tx_hash=receipt.tx_hash, - sender_address=receipt.from_address, - recipient_address=receipt.to_address, - amount=receipt.amount, - currency=model.WalletOperation.CURRENCY.ETH ) self._sci.on_transaction_confirmed(tx_hash, on_eth_receipt) return tx_hash From 9181d513cc3185a2ef6b3023417cb01257234982 Mon Sep 17 00:00:00 2001 From: Dariusz Rybi Date: Tue, 25 Jun 2019 12:24:34 +0200 Subject: [PATCH 04/10] [review] Update PaymentsKeeper docstring --- golem/ethereum/paymentskeeper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/golem/ethereum/paymentskeeper.py b/golem/ethereum/paymentskeeper.py index 0ac69ce52b..61a344e7d5 100644 --- a/golem/ethereum/paymentskeeper.py +++ b/golem/ethereum/paymentskeeper.py @@ -67,7 +67,7 @@ def get_newest_payment(num: Optional[int] = None, class PaymentsKeeper: - """Keeps information about payments for tasks + """Keeps information about outgoing payments that should be processed and send or received. """ From f8dbef5a35b2df03dd1dd13fa42f87fb4f0d7a8e Mon Sep 17 00:00:00 2001 From: Dariusz Rybi Date: Tue, 25 Jun 2019 12:46:55 +0200 Subject: [PATCH 05/10] Calculate gas_cost upon confirmation --- golem/ethereum/paymentskeeper.py | 4 ++- golem/ethereum/transactionsystem.py | 12 ++++++++ tests/factories/model.py | 1 + tests/golem/ethereum/test_paymentskeeper.py | 28 +++++++++++++------ .../golem/ethereum/test_transactionsystem.py | 4 +-- 5 files changed, 37 insertions(+), 12 deletions(-) diff --git a/golem/ethereum/paymentskeeper.py b/golem/ethereum/paymentskeeper.py index 61a344e7d5..76648e1c3c 100644 --- a/golem/ethereum/paymentskeeper.py +++ b/golem/ethereum/paymentskeeper.py @@ -76,13 +76,15 @@ def __init__(self) -> None: self.db = PaymentsDatabase() @staticmethod - def sent_transfer(tx_hash: str): + def sent_transfer(tx_hash: str, gas_amount: int, gas_price: Optional[int]): try: operation = model.WalletOperation.select() \ .where( model.WalletOperation.tx_hash == tx_hash, ).get() operation.status = model.WalletOperation.STATUS.confirmed + if gas_price is not None: + operation.gas_cost = gas_amount * gas_price operation.save() except model.WalletOperation.DoesNotExist: logger.warning( diff --git a/golem/ethereum/transactionsystem.py b/golem/ethereum/transactionsystem.py index 244e576737..b29fbf9756 100644 --- a/golem/ethereum/transactionsystem.py +++ b/golem/ethereum/transactionsystem.py @@ -316,6 +316,10 @@ def _subscribe_to_events(self) -> None: from_block=from_block, cb=lambda event: self._payments_keeper.sent_transfer( tx_hash=event.tx_hash, + gas_amount=self._sci.GAS_GNT_TRANSFER, + gas_price=self._sci.get_transaction_gas_price( + tx_hash=event.tx_hash, + ), ), ) @@ -646,6 +650,14 @@ def on_eth_receipt(receipt): return self._payments_keeper.sent_transfer( tx_hash=receipt.tx_hash, + gas_amount=self.get_withdraw_gas_cost( + amount, + destination, + currency, + ), + gas_price=self._sci.get_transaction_gas_price( + tx_hash=receipt.tx_hash, + ), ) self._sci.on_transaction_confirmed(tx_hash, on_eth_receipt) return tx_hash diff --git a/tests/factories/model.py b/tests/factories/model.py index ba7bd9733f..7a18e5c352 100644 --- a/tests/factories/model.py +++ b/tests/factories/model.py @@ -21,6 +21,7 @@ class WalletOperation(factory.Factory): class Meta: model = model.WalletOperation + status = factory.fuzzy.FuzzyChoice(model.WalletOperation.STATUS) direction = factory.fuzzy.FuzzyChoice(model.WalletOperation.DIRECTION) operation_type = factory.fuzzy.FuzzyChoice(model.WalletOperation.TYPE) sender_address = factory.LazyFunction(random_eth_address) diff --git a/tests/golem/ethereum/test_paymentskeeper.py b/tests/golem/ethereum/test_paymentskeeper.py index f8cd94f68a..ab4677932b 100644 --- a/tests/golem/ethereum/test_paymentskeeper.py +++ b/tests/golem/ethereum/test_paymentskeeper.py @@ -1,12 +1,9 @@ -from golem_messages.factories.helpers import ( - random_eth_address, -) - from golem import model from golem.ethereum.paymentskeeper import PaymentsDatabase from golem.ethereum.paymentskeeper import PaymentsKeeper from golem.tools.testwithdatabase import TestWithDatabase from tests.factories.model import TaskPayment as TaskPaymentFactory +from tests.factories.model import WalletOperation as WalletOperationFactory class TestPaymentsDatabase(TestWithDatabase): @@ -55,12 +52,25 @@ def setUp(self): self.payments_keeper = PaymentsKeeper() def test_sent_transfer(self): + operation = WalletOperationFactory() + operation.save(force_insert=True) + self.payments_keeper.sent_transfer( + tx_hash=operation.tx_hash, + gas_price=1, + gas_amount=1, + ) + self.assertEqual( + model.WalletOperation.select().count(), + 1, + ) + + def test_sent_transfer_no_price(self): + operation = WalletOperationFactory() + operation.save(force_insert=True) self.payments_keeper.sent_transfer( - tx_hash=f"0x{'0'*64}", - sender_address=random_eth_address(), - recipient_address=random_eth_address(), - amount=1, - currency=model.WalletOperation.CURRENCY.GNT, + tx_hash=operation.tx_hash, + gas_price=None, + gas_amount=1, ) self.assertEqual( model.WalletOperation.select().count(), diff --git a/tests/golem/ethereum/test_transactionsystem.py b/tests/golem/ethereum/test_transactionsystem.py index 8a9f14f8c3..0d2e190ee3 100644 --- a/tests/golem/ethereum/test_transactionsystem.py +++ b/tests/golem/ethereum/test_transactionsystem.py @@ -385,7 +385,7 @@ def test_enough_gnt(self): self.sci.convert_gntb_to_gnt.assert_called_once_with( self.dest, amount, - None, + self.ets.gas_price, ) def test_custom_gas_price_gnt(self): @@ -448,7 +448,7 @@ def test_gnt_with_lock(self): self.sci.convert_gntb_to_gnt.assert_called_once_with( self.dest, self.gnt_balance - locked_gnt, - None, + self.ets.gas_price, ) def test_not_enough_gnt_with_lock(self): From 1ae2f77a6cb720f31556dd096df5f98a504fc439 Mon Sep 17 00:00:00 2001 From: Dariusz Rybi Date: Thu, 27 Jun 2019 17:02:58 +0200 Subject: [PATCH 06/10] [review] DRY received_transfer --- golem/ethereum/incomeskeeper.py | 24 +++------------------- golem/ethereum/transactionsystem.py | 8 +++++--- tests/golem/ethereum/test_incomeskeeper.py | 6 ++++-- 3 files changed, 12 insertions(+), 26 deletions(-) diff --git a/golem/ethereum/incomeskeeper.py b/golem/ethereum/incomeskeeper.py index b06668ae18..4e97b54e03 100644 --- a/golem/ethereum/incomeskeeper.py +++ b/golem/ethereum/incomeskeeper.py @@ -16,11 +16,12 @@ class IncomesKeeper: """ @staticmethod - def received_eth_transfer( + def received_transfer( tx_hash: str, sender_address: str, recipient_address: str, amount: int, + currency, ): model.WalletOperation.create( tx_hash=tx_hash, @@ -30,26 +31,7 @@ def received_eth_transfer( sender_address=sender_address, recipient_address=recipient_address, amount=amount, - currency=model.WalletOperation.CURRENCY.ETH, - gas_cost=0, - ) - - @staticmethod - def received_gnt_transfer( - tx_hash: str, - sender_address: str, - recipient_address: str, - amount: int, - ): - model.WalletOperation.create( - tx_hash=tx_hash, - direction=model.WalletOperation.DIRECTION.incoming, - operation_type=model.WalletOperation.TYPE.transfer, - status=model.WalletOperation.STATUS.confirmed, - sender_address=sender_address, - recipient_address=recipient_address, - amount=amount, - currency=model.WalletOperation.CURRENCY.GNT, + currency=currency, gas_cost=0, ) diff --git a/golem/ethereum/transactionsystem.py b/golem/ethereum/transactionsystem.py index b29fbf9756..3af2ae6e70 100644 --- a/golem/ethereum/transactionsystem.py +++ b/golem/ethereum/transactionsystem.py @@ -290,23 +290,25 @@ def _subscribe_to_events(self) -> None: self._sci.subscribe_to_direct_incoming_eth_transfers( address=self._sci.get_eth_address(), from_block=from_block, - cb=lambda event: ik.received_eth_transfer( + cb=lambda event: ik.received_transfer( tx_hash=event.tx_hash, sender_address=event.from_address, recipient_address=event.to_address, amount=event.amount, + currency=model.WalletOperation.CURRENCY.ETH, ), ) self._sci.subscribe_to_gnt_transfers( from_address=None, to_address=self._sci.get_eth_address(), - from_blokc=from_block, - cb=lambda event: ik.received_gnt_transfer( + from_block=from_block, + cb=lambda event: ik.received_transfer( tx_hash=event.tx_hash, sender_address=event.from_address, recipient_address=event.to_address, amount=event.amount, + currency=model.WalletOperation.CURRENCY.GNT, ), ) diff --git a/tests/golem/ethereum/test_incomeskeeper.py b/tests/golem/ethereum/test_incomeskeeper.py index 803b63f55c..23bde953f2 100644 --- a/tests/golem/ethereum/test_incomeskeeper.py +++ b/tests/golem/ethereum/test_incomeskeeper.py @@ -291,11 +291,12 @@ def test_update_overdue_incomes_already_marked_as_overdue(self): ) def test_received_eth_transfer(self): - self.incomes_keeper.received_eth_transfer( + self.incomes_keeper.received_transfer( tx_hash=f"0x{'0'*64}", sender_address=random_eth_address(), recipient_address=random_eth_address(), amount=1, + currency=model.WalletOperation.CURRENCY.ETH, ) self.assertEqual( model.WalletOperation.select().count(), @@ -303,11 +304,12 @@ def test_received_eth_transfer(self): ) def test_received_gnt_transfer(self): - self.incomes_keeper.received_gnt_transfer( + self.incomes_keeper.received_transfer( tx_hash=f"0x{'0'*64}", sender_address=random_eth_address(), recipient_address=random_eth_address(), amount=1, + currency=model.WalletOperation.CURRENCY.GNT, ) self.assertEqual( model.WalletOperation.select().count(), From 08c9f4c1d56b26f9458434855bb5ceb0a6024f33 Mon Sep 17 00:00:00 2001 From: Dariusz Rybi Date: Thu, 27 Jun 2019 17:10:45 +0200 Subject: [PATCH 07/10] [review] gas_(cost|price|amount) controversy --- golem/ethereum/transactionsystem.py | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/golem/ethereum/transactionsystem.py b/golem/ethereum/transactionsystem.py index 3af2ae6e70..99a4347b43 100644 --- a/golem/ethereum/transactionsystem.py +++ b/golem/ethereum/transactionsystem.py @@ -318,7 +318,9 @@ def _subscribe_to_events(self) -> None: from_block=from_block, cb=lambda event: self._payments_keeper.sent_transfer( tx_hash=event.tx_hash, - gas_amount=self._sci.GAS_GNT_TRANSFER, + gas_amount=self._sci.get_transaction_receipt( + event.tx_hash, + ).gas_used, gas_price=self._sci.get_transaction_gas_price( tx_hash=event.tx_hash, ), @@ -652,14 +654,8 @@ def on_eth_receipt(receipt): return self._payments_keeper.sent_transfer( tx_hash=receipt.tx_hash, - gas_amount=self.get_withdraw_gas_cost( - amount, - destination, - currency, - ), - gas_price=self._sci.get_transaction_gas_price( - tx_hash=receipt.tx_hash, - ), + gas_amount=receipt.gas_cost, + gas_price=gas_price, ) self._sci.on_transaction_confirmed(tx_hash, on_eth_receipt) return tx_hash From d22987018433d580f7fa0649f407d5edc89f8f5d Mon Sep 17 00:00:00 2001 From: Dariusz Rybi Date: Thu, 27 Jun 2019 17:17:15 +0200 Subject: [PATCH 08/10] [review] rename sent_transfer to confirmed_transfer --- golem/ethereum/paymentskeeper.py | 6 +++++- golem/ethereum/transactionsystem.py | 4 ++-- tests/golem/ethereum/test_paymentskeeper.py | 8 ++++---- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/golem/ethereum/paymentskeeper.py b/golem/ethereum/paymentskeeper.py index 76648e1c3c..2a51c55d1f 100644 --- a/golem/ethereum/paymentskeeper.py +++ b/golem/ethereum/paymentskeeper.py @@ -76,7 +76,11 @@ def __init__(self) -> None: self.db = PaymentsDatabase() @staticmethod - def sent_transfer(tx_hash: str, gas_amount: int, gas_price: Optional[int]): + def confirmed_transfer( + tx_hash: str, + gas_amount: int, + gas_price: Optional[int], + ): try: operation = model.WalletOperation.select() \ .where( diff --git a/golem/ethereum/transactionsystem.py b/golem/ethereum/transactionsystem.py index 99a4347b43..46ee4afe8d 100644 --- a/golem/ethereum/transactionsystem.py +++ b/golem/ethereum/transactionsystem.py @@ -316,7 +316,7 @@ def _subscribe_to_events(self) -> None: from_address=self._sci.get_eth_address(), to_address=None, from_block=from_block, - cb=lambda event: self._payments_keeper.sent_transfer( + cb=lambda event: self._payments_keeper.confirmed_transfer( tx_hash=event.tx_hash, gas_amount=self._sci.get_transaction_receipt( event.tx_hash, @@ -652,7 +652,7 @@ def on_eth_receipt(receipt): if not receipt.status: log.error("Failed ETH withdrawal: %r", receipt) return - self._payments_keeper.sent_transfer( + self._payments_keeper.confirmed_transfer( tx_hash=receipt.tx_hash, gas_amount=receipt.gas_cost, gas_price=gas_price, diff --git a/tests/golem/ethereum/test_paymentskeeper.py b/tests/golem/ethereum/test_paymentskeeper.py index ab4677932b..fd3aae5ec1 100644 --- a/tests/golem/ethereum/test_paymentskeeper.py +++ b/tests/golem/ethereum/test_paymentskeeper.py @@ -51,10 +51,10 @@ def setUp(self): super().setUp() self.payments_keeper = PaymentsKeeper() - def test_sent_transfer(self): + def test_confirmed_transfer(self): operation = WalletOperationFactory() operation.save(force_insert=True) - self.payments_keeper.sent_transfer( + self.payments_keeper.confirmed_transfer( tx_hash=operation.tx_hash, gas_price=1, gas_amount=1, @@ -64,10 +64,10 @@ def test_sent_transfer(self): 1, ) - def test_sent_transfer_no_price(self): + def test_confirmed_transfer_no_price(self): operation = WalletOperationFactory() operation.save(force_insert=True) - self.payments_keeper.sent_transfer( + self.payments_keeper.confirmed_transfer( tx_hash=operation.tx_hash, gas_price=None, gas_amount=1, From 687f3a7854fed9b63cff222635611bc7cdf202a5 Mon Sep 17 00:00:00 2001 From: Dariusz Rybi Date: Tue, 2 Jul 2019 12:45:51 +0200 Subject: [PATCH 09/10] [review] Assume gas_price will never be None in mined transactions --- golem/ethereum/paymentskeeper.py | 6 ++-- golem/ethereum/transactionsystem.py | 36 ++++++++++++++------- tests/golem/ethereum/test_paymentskeeper.py | 16 +-------- 3 files changed, 28 insertions(+), 30 deletions(-) diff --git a/golem/ethereum/paymentskeeper.py b/golem/ethereum/paymentskeeper.py index 2a51c55d1f..6e252c64a1 100644 --- a/golem/ethereum/paymentskeeper.py +++ b/golem/ethereum/paymentskeeper.py @@ -78,8 +78,7 @@ def __init__(self) -> None: @staticmethod def confirmed_transfer( tx_hash: str, - gas_amount: int, - gas_price: Optional[int], + gas_cost: int, ): try: operation = model.WalletOperation.select() \ @@ -87,8 +86,7 @@ def confirmed_transfer( model.WalletOperation.tx_hash == tx_hash, ).get() operation.status = model.WalletOperation.STATUS.confirmed - if gas_price is not None: - operation.gas_cost = gas_amount * gas_price + operation.gas_cost = gas_cost operation.save() except model.WalletOperation.DoesNotExist: logger.warning( diff --git a/golem/ethereum/transactionsystem.py b/golem/ethereum/transactionsystem.py index 46ee4afe8d..50cce4196c 100644 --- a/golem/ethereum/transactionsystem.py +++ b/golem/ethereum/transactionsystem.py @@ -16,6 +16,7 @@ List, Optional, Tuple, + TYPE_CHECKING, ) from ethereum.utils import denoms @@ -44,6 +45,11 @@ from .faucet import tETH_faucet_donate +if TYPE_CHECKING: + # pylint: disable=unused-import,ungrouped-imports + from golem_sci import events as sci_events + + log = logging.getLogger(__name__) @@ -312,19 +318,28 @@ def _subscribe_to_events(self) -> None: ), ) + def _outgoing_gnt_transfer_confirmed( + event: 'sci_events.GntTransferEvent', + ): + assert self._sci is not None # mypy :( + receipt: TransactionReceipt = self._sci.get_transaction_receipt( + event.tx_hash, + ) + gas_price = self._sci.get_transaction_gas_price( + tx_hash=event.tx_hash, + ) + # Mined transaction won't return None + assert isinstance(gas_price, int) + self._payments_keeper.confirmed_transfer( + tx_hash=event.tx_hash, + gas_cost=receipt.gas_used * gas_price, + ) + self._sci.subscribe_to_gnt_transfers( from_address=self._sci.get_eth_address(), to_address=None, from_block=from_block, - cb=lambda event: self._payments_keeper.confirmed_transfer( - tx_hash=event.tx_hash, - gas_amount=self._sci.get_transaction_receipt( - event.tx_hash, - ).gas_used, - gas_price=self._sci.get_transaction_gas_price( - tx_hash=event.tx_hash, - ), - ), + cb=_outgoing_gnt_transfer_confirmed, ) if self.deposit_contract_available: @@ -654,8 +669,7 @@ def on_eth_receipt(receipt): return self._payments_keeper.confirmed_transfer( tx_hash=receipt.tx_hash, - gas_amount=receipt.gas_cost, - gas_price=gas_price, + gas_cost=receipt.gas_cost * gas_price, ) self._sci.on_transaction_confirmed(tx_hash, on_eth_receipt) return tx_hash diff --git a/tests/golem/ethereum/test_paymentskeeper.py b/tests/golem/ethereum/test_paymentskeeper.py index fd3aae5ec1..577082ca32 100644 --- a/tests/golem/ethereum/test_paymentskeeper.py +++ b/tests/golem/ethereum/test_paymentskeeper.py @@ -56,21 +56,7 @@ def test_confirmed_transfer(self): operation.save(force_insert=True) self.payments_keeper.confirmed_transfer( tx_hash=operation.tx_hash, - gas_price=1, - gas_amount=1, - ) - self.assertEqual( - model.WalletOperation.select().count(), - 1, - ) - - def test_confirmed_transfer_no_price(self): - operation = WalletOperationFactory() - operation.save(force_insert=True) - self.payments_keeper.confirmed_transfer( - tx_hash=operation.tx_hash, - gas_price=None, - gas_amount=1, + gas_cost=1, ) self.assertEqual( model.WalletOperation.select().count(), From 93bb04a90bdee869af5b3994996440d4cd104495 Mon Sep 17 00:00:00 2001 From: Dariusz Rybi Date: Tue, 2 Jul 2019 13:59:10 +0200 Subject: [PATCH 10/10] [review] Remove duplicated test --- tests/golem/ethereum/test_incomeskeeper.py | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/tests/golem/ethereum/test_incomeskeeper.py b/tests/golem/ethereum/test_incomeskeeper.py index 23bde953f2..634e4263b0 100644 --- a/tests/golem/ethereum/test_incomeskeeper.py +++ b/tests/golem/ethereum/test_incomeskeeper.py @@ -290,7 +290,7 @@ def test_update_overdue_incomes_already_marked_as_overdue(self): model.WalletOperation.STATUS.overdue, ) - def test_received_eth_transfer(self): + def test_received_transfer(self): self.incomes_keeper.received_transfer( tx_hash=f"0x{'0'*64}", sender_address=random_eth_address(), @@ -302,16 +302,3 @@ def test_received_eth_transfer(self): model.WalletOperation.select().count(), 1, ) - - def test_received_gnt_transfer(self): - self.incomes_keeper.received_transfer( - tx_hash=f"0x{'0'*64}", - sender_address=random_eth_address(), - recipient_address=random_eth_address(), - amount=1, - currency=model.WalletOperation.CURRENCY.GNT, - ) - self.assertEqual( - model.WalletOperation.select().count(), - 1, - )