Skip to content

Commit

Permalink
commands: fix satoshis decimal conversion in payto cmd and others
Browse files Browse the repository at this point in the history
When called via jsonrpc (but not via cli) with non-string amounts,
there could be a rounding error resulting in sending 1 sat less.

example:
```
$ ./run_electrum --testnet -w ~/.electrum/testnet/wallets/test_segwit_2 paytomany '[["tb1q6k5h4cz6ra8nzhg90xm9wldvadgh0fpttfthcg", 0.00033389]]' --fee 0
02000000000101b9e6018acb16952e3c9618b069df404dc85544eda8120e5f6e7cd7e94ce5ae8d0100000000fdffffff02fd8100000000000016001410c5b97085ec1637a9f702852f5a81f650fae1566d82000000000000160014d5a97ae05a1f4f315d0579b6577daceb5177a42b024730440220251d2ce83f6e69273de8e9be8602fbcf72b9157e1c0116161fa52f7e04db6e4302202d84045cc6b7056a215d1db3f59884e28dadd5257e1a3960068f90df90b452d1012102b0eff3bf364a2ab5effe952cba33521ebede81dac88c71951a5ed598cb48347b3a022500

$ curl --data-binary '{"id":"curltext","method":"paytomany","params":{"outputs":[["tb1q6k5h4cz6ra8nzhg90xm9wldvadgh0fpttfthcg", 0.00033389]], "fee": 0, "wallet": "/home/user/.electrum/testnet/wallets/test_segwit_2"}}' http://user:pass@127.0.0.1:7777
{"id": "curltext", "jsonrpc": "2.0", "result": "02000000000101b9e6018acb16952e3c9618b069df404dc85544eda8120e5f6e7cd7e94ce5ae8d0100000000fdffffff02fe8100000000000016001410c5b97085ec1637a9f702852f5a81f650fae1566c82000000000000160014d5a97ae05a1f4f315d0579b6577daceb5177a42b0247304402206ef66b845ca298c14dc6e8049cba9ed19db1671132194518ce5d521de6f5df8802205ca4b1aee703e3b98331fb9b88210917b385560020c8b2a8a88da38996b101c4012102b0eff3bf364a2ab5effe952cba33521ebede81dac88c71951a5ed598cb48347b39022500"}
```
^ note that first tx has output for 0.00033389, second tx has output for 0.00033388

fixes #8274
  • Loading branch information
SomberNight committed Mar 22, 2023
1 parent 0d007b5 commit 7834f6c
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 28 deletions.
30 changes: 15 additions & 15 deletions electrum/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@

from .import util, ecc
from .util import (bfh, format_satoshis, json_decode, json_normalize,
is_hash256_str, is_hex_str, to_bytes, parse_max_spend)
is_hash256_str, is_hex_str, to_bytes, parse_max_spend, to_decimal)
from . import bitcoin
from .bitcoin import is_address, hash_160, COIN
from .bip32 import BIP32Node
Expand Down Expand Up @@ -85,10 +85,10 @@ def satoshis_or_max(amount):

def satoshis(amount):
# satoshi conversion must not be performed by the parser
return int(COIN*Decimal(amount)) if amount is not None else None
return int(COIN*to_decimal(amount)) if amount is not None else None

def format_satoshis(x):
return str(Decimal(x)/COIN) if x is not None else None
return str(to_decimal(x)/COIN) if x is not None else None


class Command:
Expand Down Expand Up @@ -357,7 +357,7 @@ async def listunspent(self, wallet: Abstract_Wallet = None):
for txin in wallet.get_utxos():
d = txin.to_json()
v = d.pop("value_sats")
d["value"] = str(Decimal(v)/COIN) if v is not None else None
d["value"] = str(to_decimal(v)/COIN) if v is not None else None
coins.append(d)
return coins

Expand Down Expand Up @@ -543,13 +543,13 @@ async def getbalance(self, wallet: Abstract_Wallet = None):
"""Return the balance of your wallet. """
c, u, x = wallet.get_balance()
l = wallet.lnworker.get_balance() if wallet.lnworker else None
out = {"confirmed": str(Decimal(c)/COIN)}
out = {"confirmed": str(to_decimal(c)/COIN)}
if u:
out["unconfirmed"] = str(Decimal(u)/COIN)
out["unconfirmed"] = str(to_decimal(u)/COIN)
if x:
out["unmatured"] = str(Decimal(x)/COIN)
out["unmatured"] = str(to_decimal(x)/COIN)
if l:
out["lightning"] = str(Decimal(l)/COIN)
out["lightning"] = str(to_decimal(l)/COIN)
return out

@command('n')
Expand All @@ -559,8 +559,8 @@ async def getaddressbalance(self, address):
"""
sh = bitcoin.address_to_scripthash(address)
out = await self.network.get_balance_for_scripthash(sh)
out["confirmed"] = str(Decimal(out["confirmed"])/COIN)
out["unconfirmed"] = str(Decimal(out["unconfirmed"])/COIN)
out["confirmed"] = str(to_decimal(out["confirmed"])/COIN)
out["unconfirmed"] = str(to_decimal(out["unconfirmed"])/COIN)
return out

@command('n')
Expand Down Expand Up @@ -1056,7 +1056,7 @@ async def getfeerate(self, fee_method=None, fee_level=None):
else:
raise Exception('Invalid fee estimation method: {}'.format(fee_method))
if fee_level is not None:
fee_level = Decimal(fee_level)
fee_level = to_decimal(fee_level)
return self.config.fee_per_kb(dyn=dyn, mempool=mempool, fee_level=fee_level)

@command('w')
Expand Down Expand Up @@ -1358,7 +1358,7 @@ async def convert_currency(self, from_amount=1, from_ccy = '', to_ccy = ''):
raise Exception(f'Currency to convert to ({to_ccy}) is unknown or rate is unavailable')
# Conversion
try:
from_amount = Decimal(from_amount)
from_amount = to_decimal(from_amount)
to_amount = from_amount / rate_from * rate_to
except InvalidOperation:
raise Exception("from_amount is not a number")
Expand Down Expand Up @@ -1456,7 +1456,7 @@ def eval_bool(x: str) -> bool:

# don't use floats because of rounding errors
from .transaction import convert_raw_tx_to_hex
json_loads = lambda x: json.loads(x, parse_float=lambda x: str(Decimal(x)))
json_loads = lambda x: json.loads(x, parse_float=lambda x: str(to_decimal(x)))
arg_types = {
'num': int,
'nbits': int,
Expand All @@ -1469,8 +1469,8 @@ def eval_bool(x: str) -> bool:
'jsontx': json_loads,
'inputs': json_loads,
'outputs': json_loads,
'fee': lambda x: str(Decimal(x)) if x is not None else None,
'amount': lambda x: str(Decimal(x)) if not parse_max_spend(x) else x,
'fee': lambda x: str(to_decimal(x)) if x is not None else None,
'amount': lambda x: str(to_decimal(x)) if not parse_max_spend(x) else x,
'locktime': int,
'addtransaction': eval_bool,
'fee_method': str,
Expand Down
14 changes: 1 addition & 13 deletions electrum/exchange_rate.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from .bitcoin import COIN
from .i18n import _
from .util import (ThreadJob, make_dir, log_exceptions, OldTaskGroup,
make_aiohttp_session, resource_path, EventListener, event_listener)
make_aiohttp_session, resource_path, EventListener, event_listener, to_decimal)
from .network import Network
from .simple_config import SimpleConfig
from .logging import Logger
Expand All @@ -39,18 +39,6 @@
'BTC': 8, 'LTC': 8, 'XRP': 6, 'ETH': 18,
}


def to_decimal(x: Union[str, float, int, Decimal]) -> Decimal:
# helper function mainly for float->Decimal conversion, i.e.:
# >>> Decimal(41754.681)
# Decimal('41754.680999999996856786310672760009765625')
# >>> Decimal("41754.681")
# Decimal('41754.681')
if isinstance(x, Decimal):
return x
return Decimal(str(x))


POLL_PERIOD_SPOT_RATE = 150 # approx. every 2.5 minutes, try to refresh spot price
EXPIRY_SPOT_RATE = 600 # spot price becomes stale after 10 minutes

Expand Down
11 changes: 11 additions & 0 deletions electrum/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,17 @@ class UserCancelled(Exception):
pass


def to_decimal(x: Union[str, float, int, Decimal]) -> Decimal:
# helper function mainly for float->Decimal conversion, i.e.:
# >>> Decimal(41754.681)
# Decimal('41754.680999999996856786310672760009765625')
# >>> Decimal("41754.681")
# Decimal('41754.681')
if isinstance(x, Decimal):
return x
return Decimal(str(x))


# note: this is not a NamedTuple as then its json encoding cannot be customized
class Satoshis(object):
__slots__ = ('value',)
Expand Down

0 comments on commit 7834f6c

Please sign in to comment.