bugfix: PSBT corner cases
This commit is contained in:
scgbckbone
doc-hex
parent
15678037d5
commit
17a715bfc5
@ -4,7 +4,9 @@ This lists the new changes that have not yet been published in a normal release.
|
||||
|
||||
# Shared Improvements - Both Mk4 and Q
|
||||
|
||||
|
||||
- Bugfix: If all change outputs have `nValue=0` they're not shown in UX
|
||||
- Bugfix: Disallow negative input/output amounts in PSBT
|
||||
- Enhancement: Add warning for zero value outputs if not OP_RETURNs
|
||||
|
||||
# Mk4 Specific Changes
|
||||
|
||||
|
||||
@ -358,6 +358,7 @@ class ApproveTransaction(UserAuthorizedAction):
|
||||
#print('FatalPSBTIssue: ' + exc.args[0])
|
||||
return await self.failure(exc.args[0])
|
||||
except BaseException as exc:
|
||||
# sys.print_exception(exc)
|
||||
del self.psbt
|
||||
gc.collect()
|
||||
|
||||
@ -622,10 +623,12 @@ class ApproveTransaction(UserAuthorizedAction):
|
||||
largest_outs = []
|
||||
largest_change = []
|
||||
total_change = 0
|
||||
has_change = False
|
||||
|
||||
for idx, tx_out in self.psbt.output_iter():
|
||||
outp = self.psbt.outputs[idx]
|
||||
if outp.is_change:
|
||||
has_change = True
|
||||
total_change += tx_out.nValue
|
||||
if len(largest_change) < MAX_VISIBLE_CHANGE:
|
||||
largest_change.append((tx_out.nValue, self.chain.render_address(tx_out.scriptPubKey)))
|
||||
@ -677,7 +680,7 @@ class ApproveTransaction(UserAuthorizedAction):
|
||||
msg.write("\n")
|
||||
|
||||
# change outputs - verified to be coming back to our wallet
|
||||
if total_change > 0:
|
||||
if has_change:
|
||||
msg.write("Change back:\n%s %s\n" % self.chain.render_value(total_change))
|
||||
visible_change_sum = 0
|
||||
if len(largest_change) == 1:
|
||||
|
||||
@ -132,7 +132,7 @@ class OutptValueCache:
|
||||
|
||||
# save new addition
|
||||
assert len(key) == ENCKEY_LEN
|
||||
assert amount > 0
|
||||
# assert amount > 0
|
||||
entry = key + cls.encode_value(prevout, amount)
|
||||
cls.runtime_cache.append(entry)
|
||||
|
||||
|
||||
@ -1395,7 +1395,7 @@ class psbtObject(psbtProxy):
|
||||
assert not self.has_goc, "v0 requires exclusion of global output count"
|
||||
assert not self.has_gtv, "v0 requires exclusion of global txn version"
|
||||
assert self.txn, "v0 requires inclusion of global unsigned tx"
|
||||
assert self.txn[1] > 63, 'txn too short'
|
||||
assert self.txn[1] > 61, 'txn too short'
|
||||
assert self.fallback_locktime is None, "v0 requires exclusion of global fallback locktime"
|
||||
assert self.txn_modifiable is None, "v0 requires exclusion of global txn modifiable"
|
||||
|
||||
@ -1494,13 +1494,20 @@ class psbtObject(psbtProxy):
|
||||
num_op_return = 0
|
||||
num_op_return_size = 0
|
||||
num_unknown_scripts = 0
|
||||
zero_val_outs = 0 # only those that are not OP_RETURN are considered
|
||||
self.num_change_outputs = 0
|
||||
|
||||
for idx, txo in self.output_iter():
|
||||
output = self.outputs[idx]
|
||||
# perform output validation
|
||||
af = output.validate(idx, txo, self.my_xfp, self.active_multisig, self)
|
||||
assert txo.nValue >= 0, "negative output value: o%d" % idx
|
||||
total_out += txo.nValue
|
||||
|
||||
if (txo.nValue == 0) and (af != "op_return"):
|
||||
# OP_RETURN outputs have nValue=0 standard
|
||||
zero_val_outs += 1
|
||||
|
||||
if output.is_change:
|
||||
self.num_change_outputs += 1
|
||||
total_change += txo.nValue
|
||||
@ -1526,16 +1533,16 @@ class psbtObject(psbtProxy):
|
||||
'%s != %s' % (self.total_change_value, total_change)
|
||||
|
||||
# check fee is reasonable
|
||||
if self.total_value_out == 0:
|
||||
per_fee = 100
|
||||
else:
|
||||
the_fee = self.calculate_fee()
|
||||
if the_fee is None:
|
||||
return
|
||||
if the_fee < 0:
|
||||
raise FatalPSBTIssue("Outputs worth more than inputs!")
|
||||
the_fee = self.calculate_fee()
|
||||
if the_fee is None:
|
||||
return
|
||||
if the_fee < 0:
|
||||
raise FatalPSBTIssue("Outputs worth more than inputs!")
|
||||
|
||||
if self.total_value_out:
|
||||
per_fee = the_fee * 100 / self.total_value_out
|
||||
else:
|
||||
per_fee = 100
|
||||
|
||||
fee_limit = settings.get('fee_limit', DEFAULT_MAX_FEE_PERCENTAGE)
|
||||
|
||||
@ -1562,6 +1569,12 @@ class psbtObject(psbtProxy):
|
||||
'Sending to %d not well understood script(s).' % num_unknown_scripts)
|
||||
)
|
||||
|
||||
if zero_val_outs:
|
||||
self.warnings.append(
|
||||
('Zero Value',
|
||||
'Non-standard zero value output(s).')
|
||||
)
|
||||
|
||||
self.consolidation_tx = (self.num_change_outputs == self.num_outputs)
|
||||
|
||||
# Enforce policy related to change outputs
|
||||
@ -1703,7 +1716,7 @@ class psbtObject(psbtProxy):
|
||||
# pull out just the CTXOut object (expensive)
|
||||
utxo = inp.get_utxo(txi.prevout.n)
|
||||
|
||||
assert utxo.nValue > 0
|
||||
assert utxo.nValue >= 0, "negative input value: i%d" % i
|
||||
total_in += utxo.nValue
|
||||
|
||||
# Look at what kind of input this will be, and therefore what
|
||||
@ -1723,7 +1736,7 @@ class psbtObject(psbtProxy):
|
||||
|
||||
if not foreign:
|
||||
# no foreign inputs, we can calculate the total input value
|
||||
assert total_in > 0
|
||||
assert total_in > 0, "zero value txn"
|
||||
self.total_value_in = total_in
|
||||
else:
|
||||
# 1+ inputs don't belong to us, we can't calculate the total input value
|
||||
|
||||
@ -3135,6 +3135,97 @@ def test_null_data_op_return(fake_txn, start_sign, end_sign, reset_seed_words):
|
||||
assert "null-data" in story
|
||||
assert "OP_RETURN" in story
|
||||
|
||||
def test_smallest_txn(fake_txn, start_sign, end_sign, reset_seed_words, settings_set):
|
||||
# serialized txn has just 62 bytes and is the smallest that we support
|
||||
# 1 input (iregardless of script type) and 1 zero value null OP_RETURN
|
||||
reset_seed_words()
|
||||
settings_set('fee_limit', -1)
|
||||
psbt = fake_txn(1, 0, op_return=[(10, b"")], input_amount=10)
|
||||
start_sign(psbt, False, stxn_flags=STXN_VISUALIZE)
|
||||
story = end_sign(accept=None, expect_txn=False).decode()
|
||||
assert "null-data" in story
|
||||
assert "OP_RETURN" in story
|
||||
|
||||
|
||||
@pytest.mark.parametrize("num_outs", [1, 12])
|
||||
@pytest.mark.parametrize("change", [True, False])
|
||||
def test_zero_value_outputs(num_outs, change, fake_txn, start_sign, end_sign, reset_seed_words,
|
||||
settings_set):
|
||||
reset_seed_words()
|
||||
# user needs to disable fee limit checks to be able to do this
|
||||
settings_set("fee_limit", -1)
|
||||
change_outs = list(range(num_outs)) if change else []
|
||||
psbt = fake_txn(1, num_outs, outvals=num_outs*[0], change_outputs=change_outs, input_amount=1)
|
||||
start_sign(psbt, False, stxn_flags=STXN_VISUALIZE)
|
||||
story = end_sign(accept=None, expect_txn=False).decode()
|
||||
assert f"Zero Value: Non-standard zero value outputs: {num_outs}" in story
|
||||
assert "1 input" in story
|
||||
assert f"{num_outs} output{'' if num_outs == 1 else 's'}" in story
|
||||
assert 'Network fee 0.00000001 XTN' in story
|
||||
|
||||
if change:
|
||||
assert "0.00000000 XTN" in story.split("\n\n")[4] # change back is zero
|
||||
assert "Consolidating 0.00000000 XTN" in story
|
||||
assert "Change back" in story
|
||||
if num_outs > 1:
|
||||
assert "to addresses" in story
|
||||
else:
|
||||
assert "to address" in story
|
||||
else:
|
||||
# even
|
||||
if num_outs == 12:
|
||||
# even tho we do not see 2 outputs, fee is also 0 and 2 smaller not shown here have also value o 0
|
||||
assert story.count('0.00000000 XTN') == 12
|
||||
else:
|
||||
assert story.count('0.00000000 XTN') == 2
|
||||
assert "Change back" not in story
|
||||
|
||||
|
||||
@pytest.mark.parametrize("change", [True, False])
|
||||
@pytest.mark.parametrize("num_ins", [True, False])
|
||||
def test_zero_value_input(change, num_ins, fake_txn, start_sign, end_sign, reset_seed_words,
|
||||
cap_story):
|
||||
# 0 value inputs - not allowed
|
||||
reset_seed_words()
|
||||
psbt = fake_txn(1, 1, fee=0, input_amount=0)
|
||||
start_sign(psbt, False, stxn_flags=STXN_VISUALIZE)
|
||||
with pytest.raises(Exception):
|
||||
end_sign(accept=None, expect_txn=False).decode()
|
||||
|
||||
_, story = cap_story()
|
||||
assert "zero value txn" in story
|
||||
|
||||
|
||||
@pytest.mark.parametrize("change", [True, False])
|
||||
def test_zero_value_inputs(change, fake_txn, start_sign, end_sign, reset_seed_words):
|
||||
# one input is-non zero
|
||||
# others are zero --> allowed
|
||||
reset_seed_words()
|
||||
invals = [0 for i in range(4)] + [100]
|
||||
psbt = fake_txn(5, 1, invals=invals, outvals=[99], change_outputs=[0] if change else [], fee=20)
|
||||
start_sign(psbt, False, stxn_flags=STXN_VISUALIZE)
|
||||
end_sign(accept=None, expect_txn=False).decode()
|
||||
|
||||
|
||||
def test_negative_amount_inputs(reset_seed_words, fake_txn, start_sign, end_sign, cap_story):
|
||||
reset_seed_words()
|
||||
psbt = fake_txn(1, 1, fee=0, input_amount=-1000)
|
||||
start_sign(psbt, False, stxn_flags=STXN_VISUALIZE)
|
||||
with pytest.raises(Exception):
|
||||
end_sign(accept=None, expect_txn=False).decode()
|
||||
|
||||
_, story = cap_story()
|
||||
assert "negative input value: i0" in story
|
||||
|
||||
def test_negative_amount_outputs(reset_seed_words, fake_txn, start_sign, end_sign, cap_story):
|
||||
reset_seed_words()
|
||||
psbt = fake_txn(1, 1, outvals=[-1000], fee=0)
|
||||
start_sign(psbt, False, stxn_flags=STXN_VISUALIZE)
|
||||
with pytest.raises(Exception):
|
||||
end_sign(accept=None, expect_txn=False).decode()
|
||||
|
||||
_, story = cap_story()
|
||||
assert "negative output value: o0" in story
|
||||
|
||||
def test_mk4_done_signing_infinite_loop(goto_home, try_sign, fake_txn, enable_hw_ux,
|
||||
settings_get, is_q1):
|
||||
|
||||
Loading…
Reference in New Issue
Block a user