Skip to content

Commit f1ef7f0

Browse files
committed
Don't calculate tx fees for PSBTs with invalid money values
In decodepsbt if an invalid amount is seen, don't calculate the fee but still show the invalid value in the decode. In analyze psbt, if an invalid amount is seen, set the next step to be the creator as the creator needs to remake the transaction so that it is valid.
1 parent 3d67527 commit f1ef7f0

File tree

3 files changed

+40
-3
lines changed

3 files changed

+40
-3
lines changed

src/node/psbt.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Distributed under the MIT software license, see the accompanying
33
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
44

5+
#include <amount.h>
56
#include <coins.h>
67
#include <consensus/tx_verify.h>
78
#include <node/psbt.h>
@@ -31,6 +32,10 @@ PSBTAnalysis AnalyzePSBT(PartiallySignedTransaction psbtx)
3132
// Check for a UTXO
3233
CTxOut utxo;
3334
if (psbtx.GetInputUTXO(utxo, i)) {
35+
if (!MoneyRange(utxo.nValue) || !MoneyRange(in_amt + utxo.nValue)) {
36+
result.SetInvalid(strprintf("PSBT is not valid. Input %u has invalid value", i));
37+
return result;
38+
}
3439
in_amt += utxo.nValue;
3540
input_analysis.has_utxo = true;
3641
} else {
@@ -85,9 +90,16 @@ PSBTAnalysis AnalyzePSBT(PartiallySignedTransaction psbtx)
8590
// Get the output amount
8691
CAmount out_amt = std::accumulate(psbtx.tx->vout.begin(), psbtx.tx->vout.end(), CAmount(0),
8792
[](CAmount a, const CTxOut& b) {
93+
if (!MoneyRange(a) || !MoneyRange(b.nValue) || !MoneyRange(a + b.nValue)) {
94+
return CAmount(-1);
95+
}
8896
return a += b.nValue;
8997
}
9098
);
99+
if (!MoneyRange(out_amt)) {
100+
result.SetInvalid(strprintf("PSBT is not valid. Output amount invalid"));
101+
return result;
102+
}
91103

92104
// Get the fee
93105
CAmount fee = in_amt - out_amt;

src/rpc/rawtransaction.cpp

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1077,7 +1077,12 @@ UniValue decodepsbt(const JSONRPCRequest& request)
10771077
UniValue out(UniValue::VOBJ);
10781078

10791079
out.pushKV("amount", ValueFromAmount(txout.nValue));
1080-
total_in += txout.nValue;
1080+
if (MoneyRange(txout.nValue) && MoneyRange(total_in + txout.nValue)) {
1081+
total_in += txout.nValue;
1082+
} else {
1083+
// Hack to just not show fee later
1084+
have_all_utxos = false;
1085+
}
10811086

10821087
UniValue o(UniValue::VOBJ);
10831088
ScriptToUniv(txout.scriptPubKey, o, true);
@@ -1087,7 +1092,13 @@ UniValue decodepsbt(const JSONRPCRequest& request)
10871092
UniValue non_wit(UniValue::VOBJ);
10881093
TxToUniv(*input.non_witness_utxo, uint256(), non_wit, false);
10891094
in.pushKV("non_witness_utxo", non_wit);
1090-
total_in += input.non_witness_utxo->vout[psbtx.tx->vin[i].prevout.n].nValue;
1095+
CAmount utxo_val = input.non_witness_utxo->vout[psbtx.tx->vin[i].prevout.n].nValue;
1096+
if (MoneyRange(utxo_val) && MoneyRange(total_in + utxo_val)) {
1097+
total_in += utxo_val;
1098+
} else {
1099+
// Hack to just not show fee later
1100+
have_all_utxos = false;
1101+
}
10911102
} else {
10921103
have_all_utxos = false;
10931104
}
@@ -1203,7 +1214,12 @@ UniValue decodepsbt(const JSONRPCRequest& request)
12031214
outputs.push_back(out);
12041215

12051216
// Fee calculation
1206-
output_value += psbtx.tx->vout[i].nValue;
1217+
if (MoneyRange(psbtx.tx->vout[i].nValue) && MoneyRange(output_value + psbtx.tx->vout[i].nValue)) {
1218+
output_value += psbtx.tx->vout[i].nValue;
1219+
} else {
1220+
// Hack to just not show fee later
1221+
have_all_utxos = false;
1222+
}
12071223
}
12081224
result.pushKV("outputs", outputs);
12091225
if (have_all_utxos) {

test/functional/rpc_psbt.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -422,5 +422,14 @@ def test_psbt_input_keys(psbt_input, keys):
422422
assert_equal(analysis['next'], 'creator')
423423
assert_equal(analysis['error'], 'PSBT is not valid. Input 0 spends unspendable output')
424424

425+
self.log.info("PSBT with invalid values should have error message and Creator as next")
426+
analysis = self.nodes[0].analyzepsbt('cHNidP8BAHECAAAAAfA00BFgAm6tp86RowwH6BMImQNL5zXUcTT97XoLGz0BAAAAAAD/////AgD5ApUAAAAAFgAUKNw0x8HRctAgmvoevm4u1SbN7XL87QKVAAAAABYAFPck4gF7iL4NL4wtfRAKgQbghiTUAAAAAAABAR8AgIFq49AHABYAFJUDtxf2PHo641HEOBOAIvFMNTr2AAAA')
427+
assert_equal(analysis['next'], 'creator')
428+
assert_equal(analysis['error'], 'PSBT is not valid. Input 0 has invalid value')
429+
430+
analysis = self.nodes[0].analyzepsbt('cHNidP8BAHECAAAAAfA00BFgAm6tp86RowwH6BMImQNL5zXUcTT97XoLGz0BAAAAAAD/////AgCAgWrj0AcAFgAUKNw0x8HRctAgmvoevm4u1SbN7XL87QKVAAAAABYAFPck4gF7iL4NL4wtfRAKgQbghiTUAAAAAAABAR8A8gUqAQAAABYAFJUDtxf2PHo641HEOBOAIvFMNTr2AAAA')
431+
assert_equal(analysis['next'], 'creator')
432+
assert_equal(analysis['error'], 'PSBT is not valid. Output amount invalid')
433+
425434
if __name__ == '__main__':
426435
PSBTTest().main()

0 commit comments

Comments
 (0)