Skip to content

Commit 20c5630

Browse files
authored
Merge pull request #1277 from delta1/issues-1259
fix: assert failure on non-policy asset consolidation in CreateTransactionInternal
2 parents c248983 + e02024e commit 20c5630

File tree

8 files changed

+214
-35
lines changed

8 files changed

+214
-35
lines changed

src/rpc/rawtransaction.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -972,7 +972,7 @@ static RPCHelpMan sendrawtransaction()
972972
// will always be blinded and not explicit. In the former case, we
973973
// error out because the transaction is not blinded properly.
974974
if (!out.nNonce.IsNull() && out.nValue.IsExplicit()) {
975-
throw JSONRPCError(RPC_TRANSACTION_ERROR, "Transaction output has nonce, but is not blinded. Did you forget to call blindrawtranssaction, or rawblindrawtransaction?");
975+
throw JSONRPCError(RPC_TRANSACTION_ERROR, "Transaction output has nonce, but is not blinded. Did you forget to call blindrawtransaction, or rawblindrawtransaction?");
976976
}
977977
}
978978

src/wallet/coinselection.cpp

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,11 +308,18 @@ std::optional<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups,
308308
non_policy_effective_value += ic.effective_value;
309309
}
310310
result.AddInput(inner_result.value());
311+
} else {
312+
LogPrint(BCLog::SELECTCOINS, "Not enough funds to create target %d for asset %s\n", it->second, it->first.GetHex());
313+
return std::nullopt;
311314
}
312315
}
313316

314317
// Perform the standard Knapsack solver for the policy asset
315-
CAmount policy_target = non_policy_effective_value + mapTargetValue.at(::policyAsset);
318+
/*
319+
NOTE:
320+
CInputCoin::effective_value is negative for non-policy assets, so the sum non_policy_effective_value is negative. Therefore, it is subtracted in order to increase policy_target by the fees required.
321+
*/
322+
CAmount policy_target = mapTargetValue.at(::policyAsset) - non_policy_effective_value;
316323
if (policy_target > 0) {
317324
inner_groups.clear();
318325

@@ -346,6 +353,9 @@ std::optional<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups,
346353

347354
if (auto inner_result = KnapsackSolver(inner_groups, policy_target, ::policyAsset)) {
348355
result.AddInput(*inner_result);
356+
} else {
357+
LogPrint(BCLog::SELECTCOINS, "Not enough funds to create target %d for policy asset %s\n", policy_target, ::policyAsset.GetHex());
358+
return std::nullopt;
349359
}
350360
}
351361

src/wallet/receive.cpp

Lines changed: 29 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -52,16 +52,35 @@ bool AllInputsMine(const CWallet& wallet, const CTransaction& tx, const isminefi
5252
}
5353

5454
CAmountMap OutputGetCredit(const CWallet& wallet, const CTransaction& tx, const size_t out_index, const isminefilter& filter) {
55+
std::map<uint256, CWalletTx>::const_iterator mi = wallet.mapWallet.find(tx.GetHash());
56+
if (mi != wallet.mapWallet.end())
57+
{
58+
const CWalletTx& wtx = (*mi).second;
59+
if (out_index < wtx.tx->vout.size() && wallet.IsMine(wtx.tx->vout[out_index]) & filter) {
60+
CAmountMap amounts;
61+
amounts[wtx.GetOutputAsset(wallet, out_index)] = std::max<CAmount>(0, wtx.GetOutputValueOut(wallet, out_index));
62+
return amounts;
63+
}
64+
}
65+
return CAmountMap();
66+
}
67+
68+
CAmountMap TxGetCredit(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter) {
5569
CAmountMap nCredit;
56-
if (wallet.IsMine(tx.vout[out_index]) & filter) {
57-
CWalletTx wtx(MakeTransactionRef(std::move(tx)), TxStateInactive{});
58-
CAmount credit = std::max<CAmount>(0, wtx.GetOutputValueOut(wallet, out_index));
59-
if (!MoneyRange(credit))
60-
throw std::runtime_error(std::string(__func__) + ": value out of range");
61-
62-
nCredit[wtx.GetOutputAsset(wallet, out_index)] += credit;
63-
if (!MoneyRange(nCredit))
64-
throw std::runtime_error(std::string(__func__) + ": value out of range");
70+
{
71+
LOCK(wallet.cs_wallet);
72+
73+
for (unsigned int i = 0; i < wtx.tx->vout.size(); ++i) {
74+
if (wallet.IsMine(wtx.tx->vout[i]) & filter) {
75+
CAmount credit = std::max<CAmount>(0, wtx.GetOutputValueOut(wallet, i));
76+
if (!MoneyRange(credit))
77+
throw std::runtime_error(std::string(__func__) + ": value out of range");
78+
79+
nCredit[wtx.GetOutputAsset(wallet, i)] += credit;
80+
if (!MoneyRange(nCredit))
81+
throw std::runtime_error(std::string(__func__) + ": value out of range");
82+
}
83+
}
6584
}
6685
return nCredit;
6786
}
@@ -126,7 +145,7 @@ static CAmountMap GetCachableAmount(const CWallet& wallet, const CWalletTx& wtx,
126145
{
127146
auto& amount = wtx.m_amounts[type];
128147
if (recalculate || !amount.m_cached[filter]) {
129-
amount.Set(filter, type == CWalletTx::DEBIT ? wallet.GetDebit(*wtx.tx, filter) : TxGetCredit(wallet, *wtx.tx, filter));
148+
amount.Set(filter, type == CWalletTx::DEBIT ? wallet.GetDebit(*wtx.tx, filter) : TxGetCredit(wallet, wtx, filter));
130149
wtx.m_is_cache_empty = false;
131150
}
132151
return amount.m_value[filter];
@@ -149,26 +168,6 @@ CAmountMap CachedTxGetCredit(const CWallet& wallet, const CWalletTx& wtx, const
149168
return credit;
150169
}
151170

152-
CAmountMap TxGetCredit(const CWallet& wallet, const CTransaction& tx, const isminefilter& filter)
153-
{
154-
{
155-
LOCK(wallet.cs_wallet);
156-
std::map<uint256, CWalletTx>::const_iterator mi = wallet.mapWallet.find(tx.GetHash());
157-
if (mi != wallet.mapWallet.end())
158-
{
159-
const CWalletTx& wtx = (*mi).second;
160-
for (size_t i = 0; i < wtx.tx->vout.size(); ++i) {
161-
if (wallet.IsMine(wtx.tx->vout[i]) & filter) {
162-
CAmountMap amounts;
163-
amounts[wtx.GetOutputAsset(wallet, i)] = std::max<CAmount>(0, wtx.GetOutputValueOut(wallet, i));
164-
return amounts;
165-
}
166-
}
167-
}
168-
}
169-
return CAmountMap();
170-
}
171-
172171
CAmountMap CachedTxGetDebit(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter)
173172
{
174173
if (wtx.tx->vin.empty())

src/wallet/spend.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1529,7 +1529,12 @@ static bool CreateTransactionInternal(
15291529

15301530
// The only time that fee_needed should be less than the amount available for fees (in change_and_fee - change_amount) is when
15311531
// we are subtracting the fee from the outputs. If this occurs at any other time, it is a bug.
1532-
assert(coin_selection_params.m_subtract_fee_outputs || fee_needed <= map_change_and_fee.at(policyAsset) - change_amount);
1532+
if (!coin_selection_params.m_subtract_fee_outputs && fee_needed > map_change_and_fee.at(policyAsset) - change_amount) {
1533+
wallet.WalletLogPrintf("ERROR: not enough coins to cover for fee (needed: %d, total: %d, change: %d)\n",
1534+
fee_needed, map_change_and_fee.at(policyAsset), change_amount);
1535+
error = _("Could not cover fee");
1536+
return false;
1537+
}
15331538

15341539
// Update nFeeRet in case fee_needed changed due to dropping the change output
15351540
if (fee_needed <= map_change_and_fee.at(policyAsset) - change_amount) {

src/wallet/test/wallet_tests.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -359,8 +359,7 @@ BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup)
359359
// credit amount is calculated.
360360
wtx.MarkDirty(wallet);
361361
AddKey(wallet, coinbaseKey);
362-
// ELEMENTS: FIXME failing
363-
// BOOST_CHECK_EQUAL(CachedTxGetImmatureCredit(wallet, wtx)[CAsset()], 50*COIN);
362+
BOOST_CHECK_EQUAL(CachedTxGetImmatureCredit(wallet, wtx)[CAsset()], 50*COIN);
364363
}
365364

366365
static int64_t AddTx(ChainstateManager& chainman, CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64_t blockTime)
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
#!/usr/bin/env python3
2+
# Copyright (c) 2017-2020 The Bitcoin Core developers
3+
# Distributed under the MIT software license, see the accompanying
4+
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
5+
"""Tests reissuance functionality from the elements code tutorial
6+
See: https://elementsproject.org/elements-code-tutorial/reissuing-assets
7+
8+
TODO: add functionality from contrib/assets_tutorial/assets_tutorial.py into here
9+
"""
10+
from test_framework.blocktools import COINBASE_MATURITY
11+
from test_framework.test_framework import BitcoinTestFramework
12+
13+
class WalletTest(BitcoinTestFramework):
14+
def set_test_params(self):
15+
self.setup_clean_chain = True
16+
self.num_nodes = 2
17+
self.extra_args = [[
18+
"-blindedaddresses=1",
19+
"-initialfreecoins=2100000000000000",
20+
"-con_blocksubsidy=0",
21+
"-con_connect_genesis_outputs=1",
22+
"-txindex=1",
23+
]] * self.num_nodes
24+
self.extra_args[0].append("-anyonecanspendaremine=1")
25+
26+
def skip_test_if_missing_module(self):
27+
self.skip_if_no_wallet()
28+
29+
def run_test(self):
30+
self.generate(self.nodes[0], COINBASE_MATURITY + 1)
31+
self.sync_all()
32+
33+
assert self.nodes[0].dumpassetlabels() == {'bitcoin': 'b2e15d0d7a0c94e4e2ce0fe6e8691b9e451377f6e46e8045a86f7c4b5d4f0f23'}
34+
35+
issuance = self.nodes[0].issueasset(100, 1)
36+
asset = issuance['asset']
37+
#token = issuance['token']
38+
issuance_txid = issuance['txid']
39+
issuance_vin = issuance['vin']
40+
41+
assert len(self.nodes[0].listissuances()) == 2 # asset & reisuance token
42+
43+
self.nodes[0].generatetoaddress(1, self.nodes[0].getnewaddress(), invalid_call=False) # confirm the tx
44+
45+
issuance_addr = self.nodes[0].gettransaction(issuance_txid)['details'][0]['address']
46+
self.nodes[1].importaddress(issuance_addr)
47+
48+
issuance_key = self.nodes[0].dumpissuanceblindingkey(issuance_txid, issuance_vin)
49+
self.nodes[1].importissuanceblindingkey(issuance_txid, issuance_vin, issuance_key)
50+
issuances = self.nodes[1].listissuances()
51+
assert (issuances[0]['tokenamount'] == 1 and issuances[0]['assetamount'] == 100) \
52+
or (issuances[1]['tokenamount'] == 1 and issuances[1]['assetamount'] == 100)
53+
54+
self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1)
55+
self.generate(self.nodes[0], 10)
56+
57+
reissuance_tx = self.nodes[0].reissueasset(asset, 99)
58+
reissuance_txid = reissuance_tx['txid']
59+
issuances = self.nodes[0].listissuances(asset)
60+
assert len(issuances) == 2
61+
assert issuances[0]['isreissuance'] or issuances[1]['isreissuance']
62+
63+
self.generate(self.nodes[0], 1)
64+
65+
expected_amt = {
66+
'bitcoin': 0,
67+
'8f1560e209f6bcac318569a935a0b2513c54f326ee4820ccd5b8c1b1b4632373': 0,
68+
'4fa41f2929d4bf6975a55967d9da5b650b6b9bfddeae4d7b54b04394be328f7f': 99
69+
}
70+
assert self.nodes[0].gettransaction(reissuance_txid)['amount'] == expected_amt
71+
72+
if __name__ == '__main__':
73+
WalletTest().main()

test/functional/test_runner.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@
8888
BASE_SCRIPTS = [
8989
# Scripts that are run by default.
9090
# vv First elements tests vv
91+
'example_elements_code_tutorial.py',
9192
'feature_fedpeg.py --legacy-wallet',
9293
'feature_fedpeg.py --pre_transition --legacy-wallet',
9394
'feature_fedpeg.py --post_transition --legacy-wallet',
@@ -110,6 +111,7 @@
110111
'feature_progress.py',
111112
'rpc_getnewblockhex.py',
112113
'wallet_elements_regression_1172.py --legacy-wallet',
114+
'wallet_elements_regression_1259.py --legacy-wallet',
113115
# Longest test should go first, to favor running tests in parallel
114116
'wallet_hd.py --legacy-wallet',
115117
'wallet_hd.py --descriptors',
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
#!/usr/bin/env python3
2+
# Copyright (c) 2017-2020 The Bitcoin Core developers
3+
# Distributed under the MIT software license, see the accompanying
4+
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
5+
"""Tests that fundrawtransaction correctly handles the case of sending many
6+
inputs of an issued asset, with no policy asset recipient.
7+
8+
See: https://github.com/ElementsProject/elements/issues/1259
9+
10+
This test issues an asset and creates many utxos, which are then used as inputs in
11+
a consolidation transaction created with the various raw transaction calls.
12+
"""
13+
from decimal import Decimal
14+
15+
from test_framework.blocktools import COINBASE_MATURITY
16+
from test_framework.test_framework import BitcoinTestFramework
17+
from test_framework.util import (
18+
assert_equal,
19+
)
20+
21+
class WalletTest(BitcoinTestFramework):
22+
def set_test_params(self):
23+
self.setup_clean_chain = True
24+
self.num_nodes = 3
25+
self.extra_args = [[
26+
"-blindedaddresses=1",
27+
"-initialfreecoins=2100000000000000",
28+
"-con_blocksubsidy=0",
29+
"-con_connect_genesis_outputs=1",
30+
"-txindex=1",
31+
]] * self.num_nodes
32+
self.extra_args[0].append("-anyonecanspendaremine=1")
33+
34+
def skip_test_if_missing_module(self):
35+
self.skip_if_no_wallet()
36+
37+
def run_test(self):
38+
self.generate(self.nodes[0], COINBASE_MATURITY + 1)
39+
self.sync_all()
40+
41+
self.log.info(f"Send some policy asset to node 1")
42+
self.nodes[0].sendtoaddress(self.nodes[1].getnewaddress(), 10)
43+
self.generate(self.nodes[0], 1)
44+
45+
self.log.info(f"Issuing an asset from node 0")
46+
issuance = self.nodes[0].issueasset(1000, 1, True)
47+
self.generate(self.nodes[0], 1)
48+
asset = issuance["asset"]
49+
self.log.info(f"Asset ID is {asset}")
50+
51+
# create many outputs of the new asset on node 1
52+
num_utxos = 45
53+
value = 10
54+
fee_rate = 10
55+
self.log.info(f"Sending {num_utxos} utxos of asset to node 1")
56+
for i in range(num_utxos):
57+
self.nodes[0].sendtoaddress(self.nodes[1].getnewaddress(), value, "", "", False, False, None, None, None, asset, False, fee_rate, True)
58+
self.generate(self.nodes[0], 1)
59+
60+
# create a raw tx on node 1 consolidating the asset utxos
61+
self.log.info(f"Create the raw consolidation transaction")
62+
hex = self.nodes[1].createrawtransaction([], [{ 'asset': asset, self.nodes[2].getnewaddress(): num_utxos * value }])
63+
64+
# fund the raw tx
65+
self.log.info(f"Fund the raw transaction")
66+
raw_tx = self.nodes[1].fundrawtransaction(hex, True)
67+
68+
# blind and sign the tx
69+
self.log.info(f"Blind and sign the raw transaction")
70+
hex = self.nodes[1].blindrawtransaction(raw_tx['hex'])
71+
signed_tx = self.nodes[1].signrawtransactionwithwallet(hex)
72+
assert_equal(signed_tx['complete'], True)
73+
74+
# decode tx
75+
tx = self.nodes[1].decoderawtransaction(signed_tx['hex'])
76+
77+
assert_equal(len(tx['vin']), num_utxos + 1)
78+
assert_equal(len(tx['vout']), 3)
79+
assert_equal(tx['fee'], {'b2e15d0d7a0c94e4e2ce0fe6e8691b9e451377f6e46e8045a86f7c4b5d4f0f23': Decimal('0.00112380')}) # fee output
80+
81+
# send and mine the tx
82+
self.log.info(f"Send the raw transaction")
83+
self.nodes[1].sendrawtransaction(signed_tx['hex'])
84+
self.generate(self.nodes[1], 1)
85+
self.sync_all()
86+
balance = self.nodes[2].getbalance()
87+
assert_equal(balance[asset], Decimal(num_utxos * value))
88+
89+
90+
if __name__ == '__main__':
91+
WalletTest().main()

0 commit comments

Comments
 (0)