From d72ef784edcd59e32bec7c715ab78bf60e7a1d23 Mon Sep 17 00:00:00 2001 From: hackrush Date: Thu, 8 Feb 2018 10:24:44 +0530 Subject: [PATCH 1/3] Removed daemon side bid checks during publish Fixes #748 Fixed failing test, Yay! Why CHANGELOG? Why not anarchy? --- CHANGELOG.md | 11 +++++++++++ lbrynet/core/Wallet.py | 3 --- lbrynet/daemon/Daemon.py | 6 ------ 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f11b5f30e..c337bf0242 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,17 @@ at anytime. * exception checking in several wallet unit tests * daemon not erring properly for non-numeric values being passed to the `bid` parameter for the `publish` method * + * Fixed unnecessarily verbose exchange rate error (https://github.com/lbryio/lbry/issues/984) + * Merged two separate dht test folders into one + * Fixed value error due to a race condition when saving to the claim cache (https://github.com/lbryio/lbry/issues/1013) + * Fixed being unable to re-download updated content (#951) + * Fixed sending error messages for failed api requests + * Fixed the file manager startup being slow when handling thousands of files + * Fixed handling decryption error for blobs encrypted with an invalid key + * Fixed handling stream with no data blob (https://github.com/lbryio/lbry/issues/905) + * Fixed fetching the external ip + * Fixed API call to blob_list with --uri parameter (https://github.com/lbryio/lbry/issues/895) + * Fixed publish command to allow updating claims with bid amount higher than wallet balance(by spending the claimtrietx coin) (https://github.com/lbryio/lbry/issues/748) ### Deprecated * `channel_list_mine`, replaced with `channel_list` diff --git a/lbrynet/core/Wallet.py b/lbrynet/core/Wallet.py index c5c3a89bd4..fa512907d4 100644 --- a/lbrynet/core/Wallet.py +++ b/lbrynet/core/Wallet.py @@ -537,9 +537,6 @@ def claim_name(self, name, bid, metadata, certificate_id=None, claim_address=Non decoded = ClaimDict.load_dict(metadata) serialized = decoded.serialized - if self.get_balance() < Decimal(bid): - raise InsufficientFundsError() - claim = yield self._send_name_claim(name, serialized.encode('hex'), bid, certificate_id, claim_address, change_address) diff --git a/lbrynet/daemon/Daemon.py b/lbrynet/daemon/Daemon.py index cff92ce235..b32f958cba 100644 --- a/lbrynet/daemon/Daemon.py +++ b/lbrynet/daemon/Daemon.py @@ -715,8 +715,6 @@ def _publish_stream(self, name, bid, claim_dict, file_path=None, certificate_id= publisher = Publisher(self.session, self.lbry_file_manager, self.session.wallet, certificate_id) parse_lbry_uri(name) - if bid <= 0.0: - raise Exception("Invalid bid") if not file_path: stream_hash = yield self.storage.get_stream_hash_for_sd_hash(claim_dict['stream']['source']['source']) claim_out = yield publisher.publish_stream(name, bid, claim_dict, stream_hash, claim_address, @@ -2007,10 +2005,6 @@ def jsonrpc_publish(self, name, bid, metadata=None, file_path=None, fee=None, ti if bid <= 0.0: raise ValueError("Bid value must be greater than 0.0") - if bid >= self.session.wallet.get_balance(): - raise InsufficientFundsError('Insufficient funds. ' \ - 'Make sure you have enough LBC to deposit') - metadata = metadata or {} if fee is not None: metadata['fee'] = fee From 2368433b22947a56470031acf27731e95f4e7672 Mon Sep 17 00:00:00 2001 From: hackrush Date: Wed, 14 Feb 2018 23:31:57 +0530 Subject: [PATCH 2/3] Check for max usable balance before updating --- lbrynet/core/Wallet.py | 9 +++++++++ lbrynet/daemon/Daemon.py | 6 ++++++ lbrynet/tests/unit/core/test_Wallet.py | 2 ++ 3 files changed, 17 insertions(+) diff --git a/lbrynet/core/Wallet.py b/lbrynet/core/Wallet.py index fa512907d4..d24ef92009 100644 --- a/lbrynet/core/Wallet.py +++ b/lbrynet/core/Wallet.py @@ -537,6 +537,10 @@ def claim_name(self, name, bid, metadata, certificate_id=None, claim_address=Non decoded = ClaimDict.load_dict(metadata) serialized = decoded.serialized + amt = yield self.get_max_usable_balance_for_claim(name) + if bid > amt: + raise InsufficientFundsError() + claim = yield self._send_name_claim(name, serialized.encode('hex'), bid, certificate_id, claim_address, change_address) @@ -972,6 +976,11 @@ def _update_balance(self): lambda result: Decimal(result['confirmed']) + Decimal(result.get('unconfirmed', 0.0))) return d + @defer.inlineCallbacks + def get_max_usable_balance_for_claim(self, claim_name): + amt = yield self._run_cmd_as_defer_to_thread('get_max_spendable_amt_for_claim', claim_name) + defer.returnValue(amt) + # Always create and return a brand new address def get_new_address(self, for_change=False, account=None): return defer.succeed(self.wallet.create_new_address(account=account, diff --git a/lbrynet/daemon/Daemon.py b/lbrynet/daemon/Daemon.py index b32f958cba..4169bdf0bb 100644 --- a/lbrynet/daemon/Daemon.py +++ b/lbrynet/daemon/Daemon.py @@ -2005,6 +2005,12 @@ def jsonrpc_publish(self, name, bid, metadata=None, file_path=None, fee=None, ti if bid <= 0.0: raise ValueError("Bid value must be greater than 0.0") + amt = yield self.session.wallet.get_max_usable_balance_for_claim(name) + if bid > amt: + raise InsufficientFundsError( + "Please lower the bid value, the max amount you can specify for this claim is {}" + .format(amt)) + metadata = metadata or {} if fee is not None: metadata['fee'] = fee diff --git a/lbrynet/tests/unit/core/test_Wallet.py b/lbrynet/tests/unit/core/test_Wallet.py index 946472d2e0..c2d19e61d7 100644 --- a/lbrynet/tests/unit/core/test_Wallet.py +++ b/lbrynet/tests/unit/core/test_Wallet.py @@ -73,6 +73,8 @@ def get_name_claims(self): def _save_name_metadata(self, name, claim_outpoint, sd_hash): return defer.succeed(True) + def get_max_usable_balance_for_claim(self, name): + return defer.succeed(3) class WalletTest(unittest.TestCase): @defer.inlineCallbacks From 3754f34f5326c9454b0ad927e607536479df9da0 Mon Sep 17 00:00:00 2001 From: hackrush Date: Tue, 20 Feb 2018 09:22:15 +0530 Subject: [PATCH 3/3] Review fixes and additional comments in test --- CHANGELOG.md | 14 ++------------ lbrynet/core/Wallet.py | 7 ++++--- lbrynet/daemon/Daemon.py | 5 +++-- lbrynet/tests/unit/core/test_Wallet.py | 7 +++++-- 4 files changed, 14 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c337bf0242..59088d59a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,18 +26,8 @@ at anytime. * `get` failing with a non-useful error message when given a uri for a channel claim * exception checking in several wallet unit tests * daemon not erring properly for non-numeric values being passed to the `bid` parameter for the `publish` method - * - * Fixed unnecessarily verbose exchange rate error (https://github.com/lbryio/lbry/issues/984) - * Merged two separate dht test folders into one - * Fixed value error due to a race condition when saving to the claim cache (https://github.com/lbryio/lbry/issues/1013) - * Fixed being unable to re-download updated content (#951) - * Fixed sending error messages for failed api requests - * Fixed the file manager startup being slow when handling thousands of files - * Fixed handling decryption error for blobs encrypted with an invalid key - * Fixed handling stream with no data blob (https://github.com/lbryio/lbry/issues/905) - * Fixed fetching the external ip - * Fixed API call to blob_list with --uri parameter (https://github.com/lbryio/lbry/issues/895) - * Fixed publish command to allow updating claims with bid amount higher than wallet balance(by spending the claimtrietx coin) (https://github.com/lbryio/lbry/issues/748) + * `publish` command to allow updating claims with a `bid` amount higher than the wallet balance, so long as the amount is less than the wallet balance plus the bid amount of the claim being updated (https://github.com/lbryio/lbry/issues/748) + * ### Deprecated * `channel_list_mine`, replaced with `channel_list` diff --git a/lbrynet/core/Wallet.py b/lbrynet/core/Wallet.py index d24ef92009..e2824ec2b2 100644 --- a/lbrynet/core/Wallet.py +++ b/lbrynet/core/Wallet.py @@ -768,6 +768,9 @@ def decrypt_wallet(self): def encrypt_wallet(self, new_password, update_keyring=False): return defer.fail(NotImplementedError()) + def get_max_usable_balance_for_claim(self, claim_name): + return defer.fail(NotImplementedError()) + def _start(self): return defer.fail(NotImplementedError()) @@ -976,10 +979,8 @@ def _update_balance(self): lambda result: Decimal(result['confirmed']) + Decimal(result.get('unconfirmed', 0.0))) return d - @defer.inlineCallbacks def get_max_usable_balance_for_claim(self, claim_name): - amt = yield self._run_cmd_as_defer_to_thread('get_max_spendable_amt_for_claim', claim_name) - defer.returnValue(amt) + return self._run_cmd_as_defer_to_thread('get_max_spendable_amount_for_claim', claim_name) # Always create and return a brand new address def get_new_address(self, for_change=False, account=None): diff --git a/lbrynet/daemon/Daemon.py b/lbrynet/daemon/Daemon.py index 4169bdf0bb..53ba59c921 100644 --- a/lbrynet/daemon/Daemon.py +++ b/lbrynet/daemon/Daemon.py @@ -93,6 +93,7 @@ } SHORT_ID_LEN = 20 +MAX_UPDATE_FEE_ESTIMATE = 0.3 class IterableContainer(object): @@ -2008,8 +2009,8 @@ def jsonrpc_publish(self, name, bid, metadata=None, file_path=None, fee=None, ti amt = yield self.session.wallet.get_max_usable_balance_for_claim(name) if bid > amt: raise InsufficientFundsError( - "Please lower the bid value, the max amount you can specify for this claim is {}" - .format(amt)) + "Please lower the bid value, the maximum amount you can specify for this claim is {}" + .format(amt - MAX_UPDATE_FEE_ESTIMATE)) metadata = metadata or {} if fee is not None: diff --git a/lbrynet/tests/unit/core/test_Wallet.py b/lbrynet/tests/unit/core/test_Wallet.py index c2d19e61d7..06ad0d90de 100644 --- a/lbrynet/tests/unit/core/test_Wallet.py +++ b/lbrynet/tests/unit/core/test_Wallet.py @@ -37,7 +37,7 @@ class MocLbryumWallet(LBRYumWallet): - def __init__(self, db_dir): + def __init__(self, db_dir, max_usable_balance=3): LBRYumWallet.__init__(self, SQLiteStorage(db_dir), SimpleConfig( {"lbryum_path": db_dir, "wallet_path": os.path.join(db_dir, "testwallet")} )) @@ -46,6 +46,7 @@ def __init__(self, db_dir): self.total_reserved_points = Decimal(0.0) self.queued_payments = defaultdict(Decimal) self.network = FakeNetwork() + self._mock_max_usable_balance = max_usable_balance assert self.config.get_wallet_path() == os.path.join(self.db_dir, "testwallet") @defer.inlineCallbacks @@ -74,7 +75,9 @@ def _save_name_metadata(self, name, claim_outpoint, sd_hash): return defer.succeed(True) def get_max_usable_balance_for_claim(self, name): - return defer.succeed(3) + # The amount is returned on the basis of test_point_reservation_and_claim unittest + # Also affects test_successful_send_name_claim + return defer.succeed(self._mock_max_usable_balance) class WalletTest(unittest.TestCase): @defer.inlineCallbacks