What the developer wrote
Authored by f321x
cli: payto: fix feerate parsing
Feerate is passed to `Commands._get_fee_policy()` as str which then
tried to multiply the string by 1000. Now it first casts the string to
`Decimal` and multiplies the decimal.
Fixes https://github.com/spesmilo/electrum/issues/10315
What changed, and why it matters
This commit fixes a bug in Electrum's command-line 'payto' command where a transaction fee rate typed in as text (a string) was being incorrectly handled. The old code tried to multiply the text string by 1000, which in Python repeats the string instead of doing math, producing a wildly wrong fee policy. The fix converts the string to a number (Decimal) first, then multiplies. This could cause users who set a custom fee rate via the CLI to get an absurdly large or malformed fee value, potentially overpaying fees or having transaction creation fail.
Users relying on the Electrum CLI for payto/paytomany/sweep with custom --feerate should upgrade to the fixed version. Review any transactions created with string feerate values in prior versions for unexpectedly high fees. No immediate remote exploit, but the bug can cause financial loss via excessive fees.
Security signals we found
Incorrect string multiplication leading to malformed fee policy
CLI argument type confusion between str and numeric
Potential for unexpectedly high transaction fees or transaction creation failure
Regression test added for feerate string parsing
Evidence from the diff
In electrum/commands.py, _get_fee_policy() received fee and feerate as strings. The previous implementation did 1000 * feerate, which for a Python str performs repetition (e.g., ‘50’ * 1000 yields ‘50’ repeated 1000 times) rather than numeric multiplication. The patch casts feerate to Decimal via to_decimal() before multiplying, then converts to int to build the FeePolicy string. It also updates CLI argument type annotations from str/float to decimal for fee and feerate in payto/paytomany/sweep. A regression test verifies that passing feerate as a string produces the same transaction as the numeric path.
Changed components
electrum/commands.pytests/test_commands.pyCLI payto commandCLI paytomany commandCLI sweep commandCommands._get_fee_policy()Inspect captured patch +16 / −8
diff --git a/electrum/commands.py b/electrum/commands.py
index 03aef89..94d0d19 100644
--- a/electrum/commands.py
+++ b/electrum/commands.py
@@ -880,8 +880,8 @@ class Commands(Logger):
arg:str:privkey:Private key. Type \'?\' to get a prompt.
arg:str:destination:Bitcoin address, contact or alias
- arg:str:fee:Transaction fee (absolute, in BTC)
- arg:str:feerate:Transaction fee rate (in sat/vbyte)
+ arg:decimal:fee:Transaction fee (absolute, in BTC)
+ arg:decimal:feerate:Transaction fee rate (in sat/vbyte)
arg:int:imax:Maximum number of inputs
arg:bool:nocheck:Do not verify aliases
"""
@@ -925,15 +925,15 @@ class Commands(Logger):
message = util.to_bytes(message)
return bitcoin.verify_usermessage_with_address(address, sig, message)
- def _get_fee_policy(self, fee, feerate):
+ def _get_fee_policy(self, fee: str, feerate: str):
if fee is not None and feerate is not None:
raise Exception('Cannot set both fee and feerate')
if fee is not None:
fee_sats = satoshis(fee)
fee_policy = FeePolicy(f'fixed:{fee_sats}')
elif feerate is not None:
- feerate_per_byte = 1000 * feerate
- fee_policy = FeePolicy(f'feerate:{feerate_per_byte}')
+ sat_per_kvbyte = int(1000 * to_decimal(feerate))
+ fee_policy = FeePolicy(f'feerate:{sat_per_kvbyte}')
else:
fee_policy = FeePolicy(self.config.FEE_POLICY)
return fee_policy
@@ -946,7 +946,7 @@ class Commands(Logger):
arg:str:destination:Bitcoin address, contact or alias
arg:decimal_or_max:amount:Amount to be sent (in BTC). Type '!' to send the maximum available.
arg:decimal:fee:Transaction fee (absolute, in BTC)
- arg:float:feerate:Transaction fee rate (in sat/vbyte)
+ arg:decimal:feerate:Transaction fee rate (in sat/vbyte)
arg:str:from_addr:Source address (must be a wallet address; use sweep to spend from non-wallet address)
arg:str:change_addr:Change address. Default is a spare address, or the source address if it's not in the wallet
arg:bool:rbf:Whether to signal opt-in Replace-By-Fee in the transaction (true/false)
@@ -979,8 +979,8 @@ class Commands(Logger):
arg:json:outputs:json list of ["address", "amount in BTC"]
arg:bool:rbf:Whether to signal opt-in Replace-By-Fee in the transaction (true/false)
- arg:str:fee:Transaction fee (absolute, in BTC)
- arg:str:feerate:Transaction fee rate (in sat/vbyte)
+ arg:decimal:fee:Transaction fee (absolute, in BTC)
+ arg:decimal:feerate:Transaction fee rate (in sat/vbyte)
arg:str:from_addr:Source address (must be a wallet address; use sweep to spend from non-wallet address)
arg:str:change_addr:Change address. Default is a spare address, or the source address if it's not in the wallet
arg:bool:addtransaction:Whether transaction is to be used for broadcasting afterwards. Adds transaction to the wallet
diff --git a/tests/test_commands.py b/tests/test_commands.py
index dd6e16a..28a80a6 100644
--- a/tests/test_commands.py
+++ b/tests/test_commands.py
@@ -313,6 +313,14 @@ class TestCommandsTestnet(ElectrumTestCase):
locktime=1972344,
wallet=wallet)
+ tx_str_2 = await cmds.payto(
+ destination="tb1qsyzgpwa0vg2940u5t6l97etuvedr5dejpf9tdy",
+ amount="0.00123456",
+ feerate="50.000", # test that passing a string feerate results in the same tx
+ locktime=1972344,
+ wallet=wallet)
+
+ self.assertEqual(tx_str, tx_str_2)
tx = tx_from_any(tx_str)
self.assertEqual(2, len(tx.outputs()))
txout = TxOutput.from_address_and_value("tb1qsyzgpwa0vg2940u5t6l97etuvedr5dejpf9tdy", 123456)
Why this scored 45/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.