Skip to content

Commit 9b7ec39

Browse files
darosiordarosior
committed
wallet: use descriptor satisfaction size to estimate inputs size
Instead of using the dummysigner to compute a placeholder satisfaction, infer a descriptor on the scriptPubKey of the coin being spent and use the estimation of the satisfaction size given by the descriptor directly. Note this (almost, see next paragraph) exactly conserves the previous behaviour. For instance CalculateMaximumSignedInputSize was previously assuming the input to be spent in a transaction that spends at least one Segwit coin, since it was always accounting for the serialization of the number of witness elements. In this commit we use a placeholder for the size of the serialization of the witness stack size (1 byte). Since the logic in this commit is already tricky enough to review, and that it is only a very tiny approximation not observable through the existing tests, it is addressed in the next commit.
1 parent 8d870a9 commit 9b7ec39

File tree

6 files changed

+118
-163
lines changed

6 files changed

+118
-163
lines changed

‎src/consensus/validation.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,10 @@ static inline int64_t GetTransactionInputWeight(const CTxIn& txin)
158158
// scriptWitness size is added here because witnesses and txins are split up in segwit serialization.
159159
return ::GetSerializeSize(txin, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) * (WITNESS_SCALE_FACTOR - 1) + ::GetSerializeSize(txin, PROTOCOL_VERSION) + ::GetSerializeSize(txin.scriptWitness.stack, PROTOCOL_VERSION);
160160
}
161+
static inline int64_t GetTransactionOutputWeight(const CTxOut& txout)
162+
{
163+
return ::GetSerializeSize(txout, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) * (WITNESS_SCALE_FACTOR - 1) + ::GetSerializeSize(txout, PROTOCOL_VERSION);
164+
}
161165

162166
/** Compute at which vout of the block's coinbase transaction the witness commitment occurs, or -1 if not found */
163167
inline int GetWitnessCommitmentIndex(const CBlock& block)

‎src/test/descriptor_tests.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -152,10 +152,10 @@ void DoCheck(std::string prv, std::string pub, const std::string& norm_pub, int
152152

153153
// We must be able to estimate the max satisfaction size for any solvable descriptor top descriptor (but combo).
154154
const bool is_nontop_or_nonsolvable{!parse_priv->IsSolvable() || !parse_priv->GetOutputType()};
155-
for (const bool use_max_sig: {true, false}) {
156-
BOOST_CHECK_MESSAGE(parse_priv->MaxSatisfactionWeight(use_max_sig) || is_nontop_or_nonsolvable, prv);
157-
BOOST_CHECK_MESSAGE(parse_pub->MaxSatisfactionWeight(use_max_sig) || is_nontop_or_nonsolvable, pub);
158-
}
155+
const auto max_sat_maxsig{parse_priv->MaxSatisfactionWeight(true)};
156+
const auto max_sat_nonmaxsig{parse_priv->MaxSatisfactionWeight(true)};
157+
const bool is_input_size_info_set{max_sat_maxsig && max_sat_nonmaxsig};
158+
BOOST_CHECK_MESSAGE(is_input_size_info_set || is_nontop_or_nonsolvable, prv);
159159

160160
// The ScriptSize() must match the size of the Script string. (ScriptSize() is set for all descs but 'combo()'.)
161161
const bool is_combo{!parse_priv->IsSingleType()};

‎src/wallet/spend.cpp

Lines changed: 110 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,60 @@ using interfaces::FoundBlock;
3434
namespace wallet {
3535
static constexpr size_t OUTPUT_GROUP_MAX_ENTRIES{100};
3636

37+
/** Whether the descriptor represents, directly or not, a witness program. */
38+
static bool IsSegwit(const Descriptor& desc) {
39+
if (const auto typ = desc.GetOutputType()) return *typ != OutputType::LEGACY;
40+
return false;
41+
}
42+
43+
/** Whether to assume ECDSA signatures' will be high-r. */
44+
static bool UseMaxSig(const std::optional<CTxIn>& txin, const CCoinControl* coin_control) {
45+
// Use max sig if watch only inputs were used or if this particular input is an external input
46+
// to ensure a sufficient fee is attained for the requested feerate.
47+
return coin_control && (coin_control->fAllowWatchOnly || (txin && coin_control->IsExternalSelected(txin->prevout)));
48+
}
49+
50+
/** Get the size of an input (in witness units) once it's signed.
51+
*
52+
* @param desc The output script descriptor of the coin spent by this input.
53+
* @param txin Optionally the txin to estimate the size of. Used to determine the size of ECDSA signatures.
54+
* @param coin_control Information about the context to determine the size of ECDSA signatures.
55+
* @param tx_is_segwit Whether the transaction has at least a single input spending a segwit coin.
56+
* @param can_grind_r Whether the signer will be able to grind the R of the signature.
57+
*/
58+
static std::optional<int64_t> MaxInputWeight(const Descriptor& desc, const std::optional<CTxIn>& txin,
59+
const CCoinControl* coin_control, const bool tx_is_segwit,
60+
const bool can_grind_r) {
61+
if (const auto sat_weight = desc.MaxSatisfactionWeight(!can_grind_r || UseMaxSig(txin, coin_control))) {
62+
const bool is_segwit = IsSegwit(desc);
63+
// Account for the size of the scriptsig and the number of elements on the witness stack. Note
64+
// that if any input in the transaction is spending a witness program, we need to specify the
65+
// witness stack size for every input regardless of whether it is segwit itself.
66+
// NOTE: this also works in case of mixed scriptsig-and-witness such as in p2sh-wrapped segwit v0
67+
// outputs. In this case the size of the scriptsig length will always be one (since the redeemScript
68+
// is always a push of the witness program in this case, which is smaller than 253 bytes).
69+
const int64_t scriptsig_len = is_segwit ? 1 : GetSizeOfCompactSize(*sat_weight / WITNESS_SCALE_FACTOR);
70+
// FIXME: use the real number of stack elements instead of the "1" placeholder.
71+
const int64_t witstack_len = is_segwit ? GetSizeOfCompactSize(1) : (tx_is_segwit ? 1 : 0);
72+
// previous txid + previous vout + sequence + scriptsig len + witstack size + scriptsig or witness
73+
// NOTE: sat_weight already accounts for the witness discount accordingly.
74+
return (32 + 4 + 4 + scriptsig_len) * WITNESS_SCALE_FACTOR + witstack_len + *sat_weight;
75+
}
76+
77+
return {};
78+
}
79+
3780
int CalculateMaximumSignedInputSize(const CTxOut& txout, const COutPoint outpoint, const SigningProvider* provider, bool can_grind_r, const CCoinControl* coin_control)
3881
{
39-
CMutableTransaction txn;
40-
txn.vin.push_back(CTxIn(outpoint));
41-
if (!provider || !DummySignInput(*provider, txn.vin[0], txout, can_grind_r, coin_control)) {
42-
return -1;
82+
if (!provider) return -1;
83+
84+
if (const auto desc = InferDescriptor(txout.scriptPubKey, *provider)) {
85+
if (const auto weight = MaxInputWeight(*desc, {}, coin_control, true, can_grind_r)) {
86+
return static_cast<int>(GetVirtualTransactionSize(*weight, 0, 0));
87+
}
4388
}
44-
return GetVirtualTransactionInputSize(txn.vin[0]);
89+
90+
return -1;
4591
}
4692

4793
int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* wallet, const CCoinControl* coin_control)
@@ -50,15 +96,65 @@ int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* wallet,
5096
return CalculateMaximumSignedInputSize(txout, COutPoint(), provider.get(), wallet->CanGrindR(), coin_control);
5197
}
5298

99+
/** Infer a descriptor for the given output script. */
100+
static std::unique_ptr<Descriptor> GetDescriptor(const CWallet* wallet, const CCoinControl* coin_control,
101+
const CScript script_pubkey)
102+
{
103+
MultiSigningProvider providers;
104+
for (const auto spkman: wallet->GetScriptPubKeyMans(script_pubkey)) {
105+
providers.AddProvider(spkman->GetSolvingProvider(script_pubkey));
106+
}
107+
if (coin_control) {
108+
providers.AddProvider(std::make_unique<FlatSigningProvider>(coin_control->m_external_provider));
109+
}
110+
return InferDescriptor(script_pubkey, providers);
111+
}
112+
113+
/** Infer the maximum size of this input after it will be signed. */
114+
static std::optional<int64_t> GetSignedTxinWeight(const CWallet* wallet, const CCoinControl* coin_control,
115+
const CTxIn& txin, const CTxOut& txo, const bool tx_is_segwit,
116+
const bool can_grind_r)
117+
{
118+
// If weight was provided, use that.
119+
if (coin_control && coin_control->HasInputWeight(txin.prevout)) {
120+
return coin_control->GetInputWeight(txin.prevout);
121+
}
122+
123+
// Otherwise, use the maximum satisfaction size provided by the descriptor.
124+
std::unique_ptr<Descriptor> desc{GetDescriptor(wallet, coin_control, txo.scriptPubKey)};
125+
if (desc) return MaxInputWeight(*desc, {txin}, coin_control, tx_is_segwit, can_grind_r);
126+
127+
return {};
128+
}
129+
53130
// txouts needs to be in the order of tx.vin
54131
TxSize CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const std::vector<CTxOut>& txouts, const CCoinControl* coin_control)
55132
{
56-
CMutableTransaction txNew(tx);
57-
if (!wallet->DummySignTx(txNew, txouts, coin_control)) return TxSize{-1, -1};
58-
CTransaction ctx(txNew);
59-
int64_t vsize = GetVirtualTransactionSize(ctx);
60-
int64_t weight = GetTransactionWeight(ctx);
61-
return TxSize{vsize, weight};
133+
// nVersion + nLockTime + input count + output count
134+
int64_t weight = (4 + 4 + GetSizeOfCompactSize(tx.vin.size()) + GetSizeOfCompactSize(tx.vout.size())) * WITNESS_SCALE_FACTOR;
135+
// Whether any input spends a witness program. Necessary to run before the next loop over the
136+
// inputs in order to accurately compute the compactSize length for the witness data per input.
137+
bool is_segwit = std::any_of(txouts.begin(), txouts.end(), [&](const CTxOut& txo) {
138+
std::unique_ptr<Descriptor> desc{GetDescriptor(wallet, coin_control, txo.scriptPubKey)};
139+
if (desc) return IsSegwit(*desc);
140+
return false;
141+
});
142+
// Segwit marker and flag
143+
if (is_segwit) weight += 2;
144+
145+
// Add the size of the transaction outputs.
146+
for (const auto& txo : tx.vout) weight += GetTransactionOutputWeight(txo);
147+
148+
// Add the size of the transaction inputs as if they were signed.
149+
for (uint32_t i = 0; i < txouts.size(); i++) {
150+
const auto txin_weight = GetSignedTxinWeight(wallet, coin_control, tx.vin[i], txouts[i], is_segwit, wallet->CanGrindR());
151+
if (!txin_weight) return TxSize{-1, -1};
152+
assert(*txin_weight > -1);
153+
weight += *txin_weight;
154+
}
155+
156+
// It's ok to use 0 as the number of sigops since we never create any pathological transaction.
157+
return TxSize{GetVirtualTransactionSize(weight, 0, 0), weight};
62158
}
63159

64160
TxSize CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const CCoinControl* coin_control)
@@ -309,7 +405,9 @@ CoinsResult AvailableCoins(const CWallet& wallet,
309405
std::unique_ptr<SigningProvider> provider = wallet.GetSolvingProvider(output.scriptPubKey);
310406

311407
int input_bytes = CalculateMaximumSignedInputSize(output, COutPoint(), provider.get(), can_grind_r, coinControl);
312-
bool solvable = provider ? InferDescriptor(output.scriptPubKey, *provider)->IsSolvable() : false;
408+
// Because CalculateMaximumSignedInputSize infers a solvable descriptor to get the satisfaction size,
409+
// it is safe to assume that this input is solvable if input_bytes is greater than -1.
410+
bool solvable = input_bytes > -1;
313411
bool spendable = ((mine & ISMINE_SPENDABLE) != ISMINE_NO) || (((mine & ISMINE_WATCH_ONLY) != ISMINE_NO) && (coinControl && coinControl->fAllowWatchOnly && solvable));
314412

315413
// Filter by spendable outputs only

‎src/wallet/test/spend_tests.cpp

Lines changed: 0 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -62,57 +62,6 @@ BOOST_FIXTURE_TEST_CASE(SubtractFee, TestChain100Setup)
6262
BOOST_CHECK_EQUAL(fee, check_tx(fee + 123));
6363
}
6464

65-
static void TestFillInputToWeight(int64_t additional_weight, std::vector<int64_t> expected_stack_sizes)
66-
{
67-
static const int64_t EMPTY_INPUT_WEIGHT = GetTransactionInputWeight(CTxIn());
68-
69-
CTxIn input;
70-
int64_t target_weight = EMPTY_INPUT_WEIGHT + additional_weight;
71-
BOOST_CHECK(FillInputToWeight(input, target_weight));
72-
BOOST_CHECK_EQUAL(GetTransactionInputWeight(input), target_weight);
73-
BOOST_CHECK_EQUAL(input.scriptWitness.stack.size(), expected_stack_sizes.size());
74-
for (unsigned int i = 0; i < expected_stack_sizes.size(); ++i) {
75-
BOOST_CHECK_EQUAL(input.scriptWitness.stack[i].size(), expected_stack_sizes[i]);
76-
}
77-
}
78-
79-
BOOST_FIXTURE_TEST_CASE(FillInputToWeightTest, BasicTestingSetup)
80-
{
81-
{
82-
// Less than or equal minimum of 165 should not add any witness data
83-
CTxIn input;
84-
BOOST_CHECK(!FillInputToWeight(input, -1));
85-
BOOST_CHECK_EQUAL(GetTransactionInputWeight(input), 165);
86-
BOOST_CHECK_EQUAL(input.scriptWitness.stack.size(), 0);
87-
BOOST_CHECK(!FillInputToWeight(input, 0));
88-
BOOST_CHECK_EQUAL(GetTransactionInputWeight(input), 165);
89-
BOOST_CHECK_EQUAL(input.scriptWitness.stack.size(), 0);
90-
BOOST_CHECK(!FillInputToWeight(input, 164));
91-
BOOST_CHECK_EQUAL(GetTransactionInputWeight(input), 165);
92-
BOOST_CHECK_EQUAL(input.scriptWitness.stack.size(), 0);
93-
BOOST_CHECK(FillInputToWeight(input, 165));
94-
BOOST_CHECK_EQUAL(GetTransactionInputWeight(input), 165);
95-
BOOST_CHECK_EQUAL(input.scriptWitness.stack.size(), 0);
96-
}
97-
98-
// Make sure we can add at least one weight
99-
TestFillInputToWeight(1, {0});
100-
101-
// 1 byte compact size uint boundary
102-
TestFillInputToWeight(252, {251});
103-
TestFillInputToWeight(253, {83, 168});
104-
TestFillInputToWeight(262, {86, 174});
105-
TestFillInputToWeight(263, {260});
106-
107-
// 3 byte compact size uint boundary
108-
TestFillInputToWeight(65535, {65532});
109-
TestFillInputToWeight(65536, {21842, 43688});
110-
TestFillInputToWeight(65545, {21845, 43694});
111-
TestFillInputToWeight(65546, {65541});
112-
113-
// Note: We don't test the next boundary because of memory allocation constraints.
114-
}
115-
11665
BOOST_FIXTURE_TEST_CASE(wallet_duplicated_preset_inputs_test, TestChain100Setup)
11766
{
11867
// Verify that the wallet's Coin Selection process does not include pre-selected inputs twice in a transaction.

‎src/wallet/wallet.cpp

Lines changed: 0 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -1672,96 +1672,6 @@ void CWallet::InitWalletFlags(uint64_t flags)
16721672
if (!LoadWalletFlags(flags)) assert(false);
16731673
}
16741674

1675-
// Helper for producing a max-sized low-S low-R signature (eg 71 bytes)
1676-
// or a max-sized low-S signature (e.g. 72 bytes) depending on coin_control
1677-
bool DummySignInput(const SigningProvider& provider, CTxIn &tx_in, const CTxOut &txout, bool can_grind_r, const CCoinControl* coin_control)
1678-
{
1679-
// Fill in dummy signatures for fee calculation.
1680-
const CScript& scriptPubKey = txout.scriptPubKey;
1681-
SignatureData sigdata;
1682-
1683-
// Use max sig if watch only inputs were used, if this particular input is an external input,
1684-
// or if this wallet uses an external signer, to ensure a sufficient fee is attained for the requested feerate.
1685-
const bool use_max_sig = coin_control && (coin_control->fAllowWatchOnly || coin_control->IsExternalSelected(tx_in.prevout) || !can_grind_r);
1686-
if (!ProduceSignature(provider, use_max_sig ? DUMMY_MAXIMUM_SIGNATURE_CREATOR : DUMMY_SIGNATURE_CREATOR, scriptPubKey, sigdata)) {
1687-
return false;
1688-
}
1689-
UpdateInput(tx_in, sigdata);
1690-
return true;
1691-
}
1692-
1693-
bool FillInputToWeight(CTxIn& txin, int64_t target_weight)
1694-
{
1695-
assert(txin.scriptSig.empty());
1696-
assert(txin.scriptWitness.IsNull());
1697-
1698-
int64_t txin_weight = GetTransactionInputWeight(txin);
1699-
1700-
// Do nothing if the weight that should be added is less than the weight that already exists
1701-
if (target_weight < txin_weight) {
1702-
return false;
1703-
}
1704-
if (target_weight == txin_weight) {
1705-
return true;
1706-
}
1707-
1708-
// Subtract current txin weight, which should include empty witness stack
1709-
int64_t add_weight = target_weight - txin_weight;
1710-
assert(add_weight > 0);
1711-
1712-
// We will want to subtract the size of the Compact Size UInt that will also be serialized.
1713-
// However doing so when the size is near a boundary can result in a problem where it is not
1714-
// possible to have a stack element size and combination to exactly equal a target.
1715-
// To avoid this possibility, if the weight to add is less than 10 bytes greater than
1716-
// a boundary, the size will be split so that 2/3rds will be in one stack element, and
1717-
// the remaining 1/3rd in another. Using 3rds allows us to avoid additional boundaries.
1718-
// 10 bytes is used because that accounts for the maximum size. This does not need to be super precise.
1719-
if ((add_weight >= 253 && add_weight < 263)
1720-
|| (add_weight > std::numeric_limits<uint16_t>::max() && add_weight <= std::numeric_limits<uint16_t>::max() + 10)
1721-
|| (add_weight > std::numeric_limits<uint32_t>::max() && add_weight <= std::numeric_limits<uint32_t>::max() + 10)) {
1722-
int64_t first_weight = add_weight / 3;
1723-
add_weight -= first_weight;
1724-
1725-
first_weight -= GetSizeOfCompactSize(first_weight);
1726-
txin.scriptWitness.stack.emplace(txin.scriptWitness.stack.end(), first_weight, 0);
1727-
}
1728-
1729-
add_weight -= GetSizeOfCompactSize(add_weight);
1730-
txin.scriptWitness.stack.emplace(txin.scriptWitness.stack.end(), add_weight, 0);
1731-
assert(GetTransactionInputWeight(txin) == target_weight);
1732-
1733-
return true;
1734-
}
1735-
1736-
// Helper for producing a bunch of max-sized low-S low-R signatures (eg 71 bytes)
1737-
bool CWallet::DummySignTx(CMutableTransaction &txNew, const std::vector<CTxOut> &txouts, const CCoinControl* coin_control) const
1738-
{
1739-
// Fill in dummy signatures for fee calculation.
1740-
int nIn = 0;
1741-
const bool can_grind_r = CanGrindR();
1742-
for (const auto& txout : txouts)
1743-
{
1744-
CTxIn& txin = txNew.vin[nIn];
1745-
// If weight was provided, fill the input to that weight
1746-
if (coin_control && coin_control->HasInputWeight(txin.prevout)) {
1747-
if (!FillInputToWeight(txin, coin_control->GetInputWeight(txin.prevout))) {
1748-
return false;
1749-
}
1750-
nIn++;
1751-
continue;
1752-
}
1753-
const std::unique_ptr<SigningProvider> provider = GetSolvingProvider(txout.scriptPubKey);
1754-
if (!provider || !DummySignInput(*provider, txin, txout, can_grind_r, coin_control)) {
1755-
if (!coin_control || !DummySignInput(coin_control->m_external_provider, txin, txout, can_grind_r, coin_control)) {
1756-
return false;
1757-
}
1758-
}
1759-
1760-
nIn++;
1761-
}
1762-
return true;
1763-
}
1764-
17651675
bool CWallet::ImportScripts(const std::set<CScript> scripts, int64_t timestamp)
17661676
{
17671677
auto spk_man = GetLegacyScriptPubKeyMan();

‎src/wallet/wallet.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -650,8 +650,6 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
650650
bool SubmitTxMemoryPoolAndRelay(CWalletTx& wtx, std::string& err_string, bool relay) const
651651
EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
652652

653-
bool DummySignTx(CMutableTransaction &txNew, const std::vector<CTxOut> &txouts, const CCoinControl* coin_control = nullptr) const;
654-
655653
bool ImportScripts(const std::set<CScript> scripts, int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
656654
bool ImportPrivKeys(const std::map<CKeyID, CKey>& privkey_map, const int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
657655
bool ImportPubKeys(const std::vector<CKeyID>& ordered_pubkeys, const std::map<CKeyID, CPubKey>& pubkey_map, const std::map<CKeyID, std::pair<CPubKey, KeyOriginInfo>>& key_origins, const bool add_keypool, const bool internal, const int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
@@ -1064,10 +1062,6 @@ bool AddWalletSetting(interfaces::Chain& chain, const std::string& wallet_name);
10641062
//! Remove wallet name from persistent configuration so it will not be loaded on startup.
10651063
bool RemoveWalletSetting(interfaces::Chain& chain, const std::string& wallet_name);
10661064

1067-
bool DummySignInput(const SigningProvider& provider, CTxIn &tx_in, const CTxOut &txout, bool can_grind_r, const CCoinControl* coin_control);
1068-
1069-
bool FillInputToWeight(CTxIn& txin, int64_t target_weight);
1070-
10711065
struct MigrationResult {
10721066
std::string wallet_name;
10731067
std::shared_ptr<CWallet> watchonly_wallet;

0 commit comments

Comments
 (0)