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

Support symbolic amount/bid=everything which spends all funds from selected account/wallets. #3605

Draft
wants to merge 16 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
9cf28f0
Add an "everything" option to Transaction.create() and unit test.
moodyjon Apr 21, 2022
7bafce0
Support everything argument in Transaction.pay(), claim_create(), cla…
moodyjon Apr 26, 2022
262c30e
Support amount/bid "everything" in various JSON RPC methods as an alt…
moodyjon Apr 26, 2022
508acb3
Lint fix for transaction.py
moodyjon Apr 27, 2022
1fb3d83
Revising API away from decimal/str generalization. Use separate bool …
moodyjon Apr 28, 2022
6f12ea3
More revisions supporting bool args (bid_everything and amount_everyt…
moodyjon May 2, 2022
42708fc
Add some usages of bid_everything/amount_everything to test_claim_com…
moodyjon May 2, 2022
0fce4f8
Correct jsonrpc_account_fund handling of amount/everything.
moodyjon May 2, 2022
c1fe4ca
Correct some docstrings and regenerate api.json with scripts/generate…
moodyjon May 4, 2022
f7dfc66
Update type annotations to reflect new args and behavior of get_dewie…
moodyjon May 5, 2022
4a1b1da
Update expected error string on missing bid.
moodyjon May 18, 2022
07e8ab3
Simplify InsufficientFundsError testcase.
moodyjon Jun 7, 2022
cfefe99
Convert --amount, --amount_everything to generic amount (int or str) …
moodyjon Jun 8, 2022
8b2c284
Use the definite txo.amount (not EVERYTHING) when calling save_claims().
moodyjon Jun 9, 2022
665c12b
Use the definite txo.amount (not EVERYTHING) when calling save_suppor…
moodyjon Jun 9, 2022
6cfcb6e
In the general case, limit utxo selection to txo_type=other which exc…
moodyjon Jul 22, 2022
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
380 changes: 257 additions & 123 deletions docs/api.json

Large diffs are not rendered by default.

207 changes: 123 additions & 84 deletions lbry/extras/daemon/daemon.py

Large diffs are not rendered by default.

10 changes: 6 additions & 4 deletions lbry/wallet/account.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@
import random
from hashlib import sha256
from string import hexdigits
from typing import Type, Dict, Tuple, Optional, Any, List
from typing import Type, Dict, Tuple, Optional, Any, List, Union

from lbry.error import InvalidPasswordError
from lbry.crypto.crypt import aes_encrypt, aes_decrypt
from lbry.wallet.dewies import amount_to_dewies

from .bip32 import PrivateKey, PublicKey, KeyPath, from_extended_key_string
from .mnemonic import Mnemonic
Expand Down Expand Up @@ -526,9 +527,10 @@ def get_transactions(self, **constraints):
def get_transaction_count(self, **constraints):
return self.ledger.get_transaction_count(wallet=self.wallet, accounts=[self], **constraints)

async def fund(self, to_account, amount=None, everything=False,
async def fund(self, to_account, amount: Union[int, str],
outputs=1, broadcast=False, **constraints):
assert self.ledger == to_account.ledger, 'Can only transfer between accounts of the same ledger.'
dewies, everything = amount_to_dewies(amount)
if everything:
utxos = await self.get_utxos(**constraints)
await self.ledger.reserve_outputs(utxos)
Expand All @@ -538,13 +540,13 @@ async def fund(self, to_account, amount=None, everything=False,
funding_accounts=[self],
change_account=to_account
)
elif amount > 0:
elif dewies > 0:
to_address = await to_account.change.get_or_create_usable_address()
to_hash160 = to_account.ledger.address_to_hash160(to_address)
tx = await Transaction.create(
inputs=[],
outputs=[
Output.pay_pubkey_hash(amount//outputs, to_hash160)
Output.pay_pubkey_hash(dewies//outputs, to_hash160)
for _ in range(outputs)
],
funding_accounts=[self],
Expand Down
18 changes: 18 additions & 0 deletions lbry/wallet/dewies.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,24 @@
import textwrap
from typing import Tuple, Union
from .util import coins_to_satoshis, satoshis_to_coins

# Symbolic amount EVERYTHING
AMOUNT_EVERYTHING = "EVERYTHING"
Copy link
Member

Choose a reason for hiding this comment

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

All caps variable already means this is a constant, doesn't need a comment above it saying the same.


def amount_is_everything(amount: Union[int, str]) -> bool:
if isinstance(amount, str):
if amount != AMOUNT_EVERYTHING:
raise ValueError(f"The value '{amount}' for argument 'amount' is invalid.")
return True
elif isinstance(amount, int):
return False
else:
raise ValueError(f"The value '{amount}' for argument 'amount' is invalid.")

def amount_to_dewies(amount: Union[int, str]) -> Tuple[int, bool]:
everything = amount_is_everything(amount)
dewies = 0 if everything else amount
return dewies, everything

def lbc_to_dewies(lbc: str) -> int:
try:
Expand Down
83 changes: 67 additions & 16 deletions lbry/wallet/transaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import logging
import typing
from binascii import hexlify, unhexlify
from typing import List, Iterable, Optional, Tuple
from typing import List, Iterable, Optional, Tuple, Union

from lbry.error import InsufficientFundsError
from lbry.crypto.hash import hash160, sha256
Expand All @@ -12,6 +12,7 @@
from lbry.schema.base import Signable
from lbry.schema.purchase import Purchase
from lbry.schema.support import Support
from lbry.wallet.dewies import amount_to_dewies

from .script import InputScript, OutputScript
from .constants import COIN, DUST, NULL_HASH32
Expand Down Expand Up @@ -793,7 +794,8 @@ def ensure_all_have_same_ledger_and_wallet(
@classmethod
async def create(cls, inputs: Iterable[Input], outputs: Iterable[Output],
funding_accounts: Iterable['Account'], change_account: 'Account',
sign: bool = True):
sign: bool = True,
*, everything: bool = False):
""" Find optimal set of inputs when only outputs are provided; add change
outputs if only inputs are provided or if inputs are greater than outputs. """

Expand All @@ -803,6 +805,20 @@ async def create(cls, inputs: Iterable[Input], outputs: Iterable[Output],

ledger, _ = cls.ensure_all_have_same_ledger_and_wallet(funding_accounts, change_account)

if everything and not any(map(lambda txi: not txi.txo_ref.txo.is_claim, tx._inputs)):
# Spend "everything" requested, but inputs not specified.
# Make a set of inputs from all funding accounts.
all_utxos = []
for acct in funding_accounts:
# TODO: Constraints for get_utxos()?
utxos = await acct.get_utxos()
Copy link
Member

Choose a reason for hiding this comment

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

UTXOs will include claims

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On the jsonrpc_account_fund() code path, it seems the only extraneous arg which could be interpreted as a constraint is from_account=None. Will investigate this...

Possibly this imposes a constraint to exclude claims?

def constraint_spending_utxos(constraints):

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TXO_TYPES = {
    "other": 0,
    "stream": 1,
    "channel": 2,
    "support": 3,
    "purchase": 4,
    "collection": 5,
    "repost": 6,
}

    def constraint_spending_utxos(constraints):
        constraints['txo_type__in'] = (0, TXO_TYPES['purchase'])

More context:
7d333ef

I take it "other" is a generic txo carrying an amount of LBC created by wallet_send, and "purchase" is a purchase receipt txo created by purchase_create. It is unclear there is any non-zero amount associated with a purchase receipt txo. Is this correct?

The intention of the original bug was to send only "other" funds, NOT purchase receipts (and NOT the claim types as you say). Transferring both "other" and "purchase" inside jsonrpc_account_fund() kind of makes sense because in that context both from/to accounts are in the same wallet.

I will work to exclude "purchase" txos from funds transferred by support_create, channel_create, stream_create, etc.

await acct.ledger.reserve_outputs(utxos)
all_utxos.extend(utxos)
if not all_utxos:
raise InsufficientFundsError()
everything_in = [Input.spend(txo) for txo in all_utxos]
tx.add_inputs(everything_in)

# value of the outputs plus associated fees
cost = (
tx.get_base_fee(ledger) +
Expand All @@ -811,6 +827,17 @@ async def create(cls, inputs: Iterable[Input], outputs: Iterable[Output],
# value of the inputs less the cost to spend those inputs
payment = tx.get_effective_input_sum(ledger)

if everything and tx._outputs and payment > cost:
# Distribute the surplus across the known set of outputs.
amount = (payment - cost) // len(tx._outputs)
for txo in tx._outputs:
txo.amount += amount
# Recompute: value of the outputs plus associated fees
cost = (
tx.get_base_fee(ledger) +
tx.get_total_output_sum(ledger)
)

try:

for _ in range(5):
Expand Down Expand Up @@ -889,65 +916,89 @@ async def sign(self, funding_accounts: Iterable['Account'], extra_keys: dict = N
self._reset()

@classmethod
def pay(cls, amount: int, address: bytes, funding_accounts: List['Account'], change_account: 'Account'):
def pay(cls, amount: Union[int, str], addresses: List[bytes],
funding_accounts: List['Account'], change_account: 'Account'):
ledger, _ = cls.ensure_all_have_same_ledger_and_wallet(funding_accounts, change_account)
output = Output.pay_pubkey_hash(amount, ledger.address_to_hash160(address))
return cls.create([], [output], funding_accounts, change_account)
dewies, everything = amount_to_dewies(amount)
outputs = []
for address in addresses:
if ledger.is_pubkey_address(address):
outputs.append(
Output.pay_pubkey_hash(
dewies, ledger.address_to_hash160(address)
)
)
elif ledger.is_script_address(address):
outputs.append(
Output.pay_script_hash(
dewies, ledger.address_to_hash160(address)
)
)
else:
raise ValueError(f"Unsupported address: '{address}'") # TODO: use error from lbry.error
return cls.create([], outputs, funding_accounts, change_account, everything=everything)

@classmethod
def claim_create(
cls, name: str, claim: Claim, amount: int, holding_address: str,
cls, name: str, claim: Claim, amount: Union[int, str], holding_address: str,
funding_accounts: List['Account'], change_account: 'Account', signing_channel: Output = None):
ledger, _ = cls.ensure_all_have_same_ledger_and_wallet(funding_accounts, change_account)
dewies, everything = amount_to_dewies(amount)
claim_output = Output.pay_claim_name_pubkey_hash(
amount, name, claim, ledger.address_to_hash160(holding_address)
dewies, name, claim, ledger.address_to_hash160(holding_address)
)
if signing_channel is not None:
claim_output.sign(signing_channel, b'placeholder txid:nout')
return cls.create([], [claim_output], funding_accounts, change_account, sign=False)
return cls.create([], [claim_output], funding_accounts, change_account,
sign=False, everything=everything)

@classmethod
def claim_update(
cls, previous_claim: Output, claim: Claim, amount: int, holding_address: str,
cls, previous_claim: Output, claim: Claim, amount: Union[int, str], holding_address: str,
funding_accounts: List['Account'], change_account: 'Account', signing_channel: Output = None):
ledger, _ = cls.ensure_all_have_same_ledger_and_wallet(funding_accounts, change_account)
dewies, everything = amount_to_dewies(amount)
updated_claim = Output.pay_update_claim_pubkey_hash(
amount, previous_claim.claim_name, previous_claim.claim_id,
dewies, previous_claim.claim_name, previous_claim.claim_id,
claim, ledger.address_to_hash160(holding_address)
)
if signing_channel is not None:
updated_claim.sign(signing_channel, b'placeholder txid:nout')
else:
updated_claim.clear_signature()
return cls.create(
[Input.spend(previous_claim)], [updated_claim], funding_accounts, change_account, sign=False
[Input.spend(previous_claim)], [updated_claim], funding_accounts, change_account,
sign=False, everything=everything
)

@classmethod
def support(cls, claim_name: str, claim_id: str, amount: int, holding_address: str,
def support(cls, claim_name: str, claim_id: str, amount: Union[int, str], holding_address: str,
funding_accounts: List['Account'], change_account: 'Account', signing_channel: Output = None,
comment: str = None):
ledger, _ = cls.ensure_all_have_same_ledger_and_wallet(funding_accounts, change_account)
dewies, everything = amount_to_dewies(amount)
if signing_channel is not None or comment is not None:
support = Support()
if comment is not None:
support.comment = comment
support_output = Output.pay_support_data_pubkey_hash(
amount, claim_name, claim_id, support, ledger.address_to_hash160(holding_address)
dewies, claim_name, claim_id, support, ledger.address_to_hash160(holding_address)
)
if signing_channel is not None:
support_output.sign(signing_channel, b'placeholder txid:nout')
else:
support_output = Output.pay_support_pubkey_hash(
amount, claim_name, claim_id, ledger.address_to_hash160(holding_address)
dewies, claim_name, claim_id, ledger.address_to_hash160(holding_address)
)
return cls.create([], [support_output], funding_accounts, change_account, sign=False)
return cls.create([], [support_output], funding_accounts, change_account,
sign=False, everything=everything)

@classmethod
def purchase(cls, claim_id: str, amount: int, merchant_address: bytes,
funding_accounts: List['Account'], change_account: 'Account'):
ledger, _ = cls.ensure_all_have_same_ledger_and_wallet(funding_accounts, change_account)
payment = Output.pay_pubkey_hash(amount, ledger.address_to_hash160(merchant_address))
dewies, _ = amount_to_dewies(amount)
payment = Output.pay_pubkey_hash(dewies, ledger.address_to_hash160(merchant_address))
data = Output.add_purchase_data(Purchase(claim_id))
return cls.create([], [payment, data], funding_accounts, change_account)

Expand Down
57 changes: 48 additions & 9 deletions tests/integration/claims/test_claim_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -1125,23 +1125,31 @@ async def test_channel_bids(self):
tx = await self.channel_update(claim_id)
self.assertEqual(tx['outputs'][0]['amount'], '5.0')

# bid changed on update
# spend exactly amount available, no change
tx = await self.channel_update(claim_id, bid_everything=True)
await self.assertBalance(self.account, '0.0')
self.assertEqual(len(tx['outputs']), 1) # no change
self.assertEqual(tx['outputs'][0]['amount'], '9.991457')
self.assertItemCount(await self.daemon.jsonrpc_channel_list(), 1)
Copy link
Member

Choose a reason for hiding this comment

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

Can you move this to the end of the test, this way you don't have to change the values of the other asserts.


# bid reduced on update
tx = await self.channel_update(claim_id, bid='4.0')
self.assertEqual(tx['outputs'][0]['amount'], '4.0')

await self.assertBalance(self.account, '5.991503')
await self.assertBalance(self.account, '5.991299')

# not enough funds
with self.assertRaisesRegex(
InsufficientFundsError, "Not enough funds to cover this transaction."):
await self.channel_create('@foo2', '9.0')
self.assertItemCount(await self.daemon.jsonrpc_channel_list(), 1)
await self.assertBalance(self.account, '5.991503')
await self.assertBalance(self.account, '5.991299')

# spend exactly amount available, no change
tx = await self.channel_create('@foo3', '5.981322')
tx = await self.channel_create('@foo3', bid=None, bid_everything=True)
await self.assertBalance(self.account, '0.0')
self.assertEqual(len(tx['outputs']), 1) # no change
self.assertEqual(tx['outputs'][0]['amount'], '5.98122')
self.assertItemCount(await self.daemon.jsonrpc_channel_list(), 2)

async def test_setting_channel_fields(self):
Expand Down Expand Up @@ -1337,23 +1345,31 @@ async def test_stream_bids(self):
tx = await self.stream_update(claim_id)
self.assertEqual(tx['outputs'][0]['amount'], '2.0')

# bid changed on update
# spend exactly amount available, no change
tx = await self.stream_update(claim_id, bid_everything=True)
await self.assertBalance(self.account, '0.0')
self.assertEqual(len(tx['outputs']), 1) # no change
self.assertEqual(tx['outputs'][0]['amount'], '9.993347')
self.assertItemCount(await self.daemon.jsonrpc_claim_list(), 1)

# bid reduced on update
tx = await self.stream_update(claim_id, bid='3.0')
self.assertEqual(tx['outputs'][0]['amount'], '3.0')

await self.assertBalance(self.account, '6.993319')
await self.assertBalance(self.account, '6.993134')

# not enough funds
with self.assertRaisesRegex(
InsufficientFundsError, "Not enough funds to cover this transaction."):
await self.stream_create('foo2', '9.0')
self.assertItemCount(await self.daemon.jsonrpc_claim_list(), 1)
await self.assertBalance(self.account, '6.993319')
await self.assertBalance(self.account, '6.993134')

# spend exactly amount available, no change
tx = await self.stream_create('foo3', '6.98523')
tx = await self.stream_create('foo3', bid=None, bid_everything=True)
await self.assertBalance(self.account, '0.0')
self.assertEqual(len(tx['outputs']), 1) # no change
self.assertEqual(tx['outputs'][0]['amount'], '6.985055')
self.assertItemCount(await self.daemon.jsonrpc_claim_list(), 2)

async def test_stream_update_and_abandon_across_accounts(self):
Expand Down Expand Up @@ -2113,7 +2129,7 @@ async def test_abandoning_stream_at_loss(self):
async def test_publish(self):

# errors on missing arguments to create a stream
with self.assertRaisesRegex(Exception, "'bid' is a required argument for new publishes."):
with self.assertRaisesRegex(Exception, "None or null is not valid value for argument 'bid'."):
await self.daemon.jsonrpc_publish('foo')

# successfully create stream
Expand Down Expand Up @@ -2271,9 +2287,32 @@ async def test_regular_supports_and_tip_supports(self):
self.assertEqual(txs2[0]['value'], '0.0')
self.assertEqual(txs2[0]['fee'], '-0.0001415')

# send all remaining funds to support the claim using account2
support = await self.out(
self.daemon.jsonrpc_support_create(
claim_id, amount=None, tip=False, account_id=account2.id, wallet_id='wallet2',
funding_account_ids=[account2.id], amount_everything=True, blocking=True)
)
await self.confirm_tx(support['txid'])

# account2 balance went down to 0.0
await self.assertBalance(self.account, '3.979769')
await self.assertBalance(account2, '0.0')

# verify that the outgoing support is marked correctly as is_tip=False in account2
txs2 = await self.transaction_list(wallet_id='wallet2')
self.assertEqual(len(txs2[0]['support_info']), 1)
self.assertEqual(txs2[0]['support_info'][0]['balance_delta'], '-1.9996035')
self.assertEqual(txs2[0]['support_info'][0]['claim_id'], claim_id)
self.assertFalse(txs2[0]['support_info'][0]['is_tip'])
self.assertFalse(txs2[0]['support_info'][0]['is_spent'])
self.assertEqual(txs2[0]['value'], '0.0')
self.assertEqual(txs2[0]['fee'], '-0.0001135')

# abandoning the tip increases balance and shows tip as spent
await self.support_abandon(claim_id)
await self.assertBalance(self.account, '4.979662')
await self.assertBalance(account2, '0.0')
txs = await self.transaction_list(account_id=self.account.id)
self.assertEqual(len(txs[0]['abandon_info']), 1)
self.assertEqual(len(txs[1]['support_info']), 1)
Expand Down