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

Include existing claim amount in the max biddable amount for a publish #1108

Merged
merged 4 commits into from
Feb 22, 2018
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +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
*
* `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`
Expand Down
9 changes: 8 additions & 1 deletion lbrynet/core/Wallet.py
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,8 @@ 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):
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'),
Expand Down Expand Up @@ -767,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())

Expand Down Expand Up @@ -975,6 +979,9 @@ def _update_balance(self):
lambda result: Decimal(result['confirmed']) + Decimal(result.get('unconfirmed', 0.0)))
return d

def get_max_usable_balance_for_claim(self, claim_name):
Copy link
Member

Choose a reason for hiding this comment

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

Please add a matching dummy method in Wallet too

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!

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):
return defer.succeed(self.wallet.create_new_address(account=account,
Expand Down
11 changes: 6 additions & 5 deletions lbrynet/daemon/Daemon.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@
}

SHORT_ID_LEN = 20
MAX_UPDATE_FEE_ESTIMATE = 0.3


class IterableContainer(object):
Expand Down Expand Up @@ -715,8 +716,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:
Copy link
Member

Choose a reason for hiding this comment

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

The reason this is here is because otherwise we have to possibly make a stream before lbryum would tell us something is wrong

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,
Expand Down Expand Up @@ -2007,9 +2006,11 @@ 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. ' \
Copy link
Member

Choose a reason for hiding this comment

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

Same reason as the other bid check

Copy link
Member

Choose a reason for hiding this comment

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

Also, this should be formatted as something like:

raise InsufficientFundsError(
  'Insufficient funds. Make sure you have enough LBC to deposit.'
)

Or perhaps, if the message is constant it should just be built-in to the InsufficientFundsError. How many different messages do we need for this exception?

'Make sure you have enough LBC to deposit')
amt = yield self.session.wallet.get_max_usable_balance_for_claim(name)
if bid > amt:
raise InsufficientFundsError(
"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:
Expand Down
7 changes: 6 additions & 1 deletion lbrynet/tests/unit/core/test_Wallet.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")}
))
Expand All @@ -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
Expand Down Expand Up @@ -73,6 +74,10 @@ 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):
# 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
Expand Down