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.

Github-Pull: #17156
Rebased-From: f1ef7f0aa46338f4cd8de79696027a1bf868f359
This commit is contained in:
Andrew Chow 2019-10-15 17:15:22 -04:00 committed by Luke Dashjr
parent 178a834687
commit 1cf77a2dc3
3 changed files with 40 additions and 3 deletions

View file

@ -2,6 +2,7 @@
// Distributed under the MIT software license, see the accompanying // Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php. // file COPYING or http://www.opensource.org/licenses/mit-license.php.
#include <amount.h>
#include <coins.h> #include <coins.h>
#include <consensus/tx_verify.h> #include <consensus/tx_verify.h>
#include <node/psbt.h> #include <node/psbt.h>
@ -31,6 +32,10 @@ PSBTAnalysis AnalyzePSBT(PartiallySignedTransaction psbtx)
// Check for a UTXO // Check for a UTXO
CTxOut utxo; CTxOut utxo;
if (psbtx.GetInputUTXO(utxo, i)) { if (psbtx.GetInputUTXO(utxo, i)) {
if (!MoneyRange(utxo.nValue) || !MoneyRange(in_amt + utxo.nValue)) {
result.SetInvalid(strprintf("PSBT is not valid. Input %u has invalid value", i));
return result;
}
in_amt += utxo.nValue; in_amt += utxo.nValue;
input_analysis.has_utxo = true; input_analysis.has_utxo = true;
} else { } else {
@ -85,9 +90,16 @@ PSBTAnalysis AnalyzePSBT(PartiallySignedTransaction psbtx)
// Get the output amount // Get the output amount
CAmount out_amt = std::accumulate(psbtx.tx->vout.begin(), psbtx.tx->vout.end(), CAmount(0), CAmount out_amt = std::accumulate(psbtx.tx->vout.begin(), psbtx.tx->vout.end(), CAmount(0),
[](CAmount a, const CTxOut& b) { [](CAmount a, const CTxOut& b) {
if (!MoneyRange(a) || !MoneyRange(b.nValue) || !MoneyRange(a + b.nValue)) {
return CAmount(-1);
}
return a += b.nValue; return a += b.nValue;
} }
); );
if (!MoneyRange(out_amt)) {
result.SetInvalid(strprintf("PSBT is not valid. Output amount invalid"));
return result;
}
// Get the fee // Get the fee
CAmount fee = in_amt - out_amt; CAmount fee = in_amt - out_amt;

View file

@ -1074,7 +1074,12 @@ UniValue decodepsbt(const JSONRPCRequest& request)
UniValue out(UniValue::VOBJ); UniValue out(UniValue::VOBJ);
out.pushKV("amount", ValueFromAmount(txout.nValue)); out.pushKV("amount", ValueFromAmount(txout.nValue));
total_in += txout.nValue; if (MoneyRange(txout.nValue) && MoneyRange(total_in + txout.nValue)) {
total_in += txout.nValue;
} else {
// Hack to just not show fee later
have_all_utxos = false;
}
UniValue o(UniValue::VOBJ); UniValue o(UniValue::VOBJ);
ScriptToUniv(txout.scriptPubKey, o, true); ScriptToUniv(txout.scriptPubKey, o, true);
@ -1084,7 +1089,13 @@ UniValue decodepsbt(const JSONRPCRequest& request)
UniValue non_wit(UniValue::VOBJ); UniValue non_wit(UniValue::VOBJ);
TxToUniv(*input.non_witness_utxo, uint256(), non_wit, false); TxToUniv(*input.non_witness_utxo, uint256(), non_wit, false);
in.pushKV("non_witness_utxo", non_wit); in.pushKV("non_witness_utxo", non_wit);
total_in += input.non_witness_utxo->vout[psbtx.tx->vin[i].prevout.n].nValue; CAmount utxo_val = input.non_witness_utxo->vout[psbtx.tx->vin[i].prevout.n].nValue;
if (MoneyRange(utxo_val) && MoneyRange(total_in + utxo_val)) {
total_in += utxo_val;
} else {
// Hack to just not show fee later
have_all_utxos = false;
}
} else { } else {
have_all_utxos = false; have_all_utxos = false;
} }
@ -1200,7 +1211,12 @@ UniValue decodepsbt(const JSONRPCRequest& request)
outputs.push_back(out); outputs.push_back(out);
// Fee calculation // Fee calculation
output_value += psbtx.tx->vout[i].nValue; if (MoneyRange(psbtx.tx->vout[i].nValue) && MoneyRange(output_value + psbtx.tx->vout[i].nValue)) {
output_value += psbtx.tx->vout[i].nValue;
} else {
// Hack to just not show fee later
have_all_utxos = false;
}
} }
result.pushKV("outputs", outputs); result.pushKV("outputs", outputs);
if (have_all_utxos) { if (have_all_utxos) {

View file

@ -421,5 +421,14 @@ class PSBTTest(BitcoinTestFramework):
assert_equal(analysis['next'], 'creator') assert_equal(analysis['next'], 'creator')
assert_equal(analysis['error'], 'PSBT is not valid. Input 0 spends unspendable output') assert_equal(analysis['error'], 'PSBT is not valid. Input 0 spends unspendable output')
self.log.info("PSBT with invalid values should have error message and Creator as next")
analysis = self.nodes[0].analyzepsbt('cHNidP8BAHECAAAAAfA00BFgAm6tp86RowwH6BMImQNL5zXUcTT97XoLGz0BAAAAAAD/////AgD5ApUAAAAAFgAUKNw0x8HRctAgmvoevm4u1SbN7XL87QKVAAAAABYAFPck4gF7iL4NL4wtfRAKgQbghiTUAAAAAAABAR8AgIFq49AHABYAFJUDtxf2PHo641HEOBOAIvFMNTr2AAAA')
assert_equal(analysis['next'], 'creator')
assert_equal(analysis['error'], 'PSBT is not valid. Input 0 has invalid value')
analysis = self.nodes[0].analyzepsbt('cHNidP8BAHECAAAAAfA00BFgAm6tp86RowwH6BMImQNL5zXUcTT97XoLGz0BAAAAAAD/////AgCAgWrj0AcAFgAUKNw0x8HRctAgmvoevm4u1SbN7XL87QKVAAAAABYAFPck4gF7iL4NL4wtfRAKgQbghiTUAAAAAAABAR8A8gUqAQAAABYAFJUDtxf2PHo641HEOBOAIvFMNTr2AAAA')
assert_equal(analysis['next'], 'creator')
assert_equal(analysis['error'], 'PSBT is not valid. Output amount invalid')
if __name__ == '__main__': if __name__ == '__main__':
PSBTTest().main() PSBTTest().main()