aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorAntoine Poinsot <darosior@protonmail.com>2022-11-25 10:45:55 +0100
committerAntoine Poinsot <darosior@protonmail.com>2023-08-25 12:40:12 +0200
commit10546a569c6c96a5ec1b9708abf9ff5c8644f669 (patch)
tree51942798a796e8d8b2c8c6c0f61d7b9bb0e3c3d7 /src
parent9b7ec393b82ca9d7ada77d06e0835df0386a8b85 (diff)
downloadbitcoin-10546a569c6c96a5ec1b9708abf9ff5c8644f669.tar.xz
wallet: accurately account for the size of the witness stack
When estimating the maximum size of an input, we were assuming the number of elements on the witness stack could be encode in a single byte. This is a valid approximation for all the descriptors we support (including P2WSH Miniscript ones), but may not hold anymore once we support Miniscript within Taproot descriptors (since the max standard witness stack size of 100 gets lifted). It's a low-hanging fruit to account for it correctly, so just do it now.
Diffstat (limited to 'src')
-rw-r--r--src/script/descriptor.cpp36
-rw-r--r--src/script/descriptor.h3
-rw-r--r--src/test/descriptor_tests.cpp3
-rw-r--r--src/test/fuzz/descriptor_parse.cpp3
-rw-r--r--src/wallet/spend.cpp27
-rw-r--r--src/wallet/test/walletload_tests.cpp1
6 files changed, 58 insertions, 15 deletions
diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp
index 727cf6118f..2f3f2c7a1d 100644
--- a/src/script/descriptor.cpp
+++ b/src/script/descriptor.cpp
@@ -718,6 +718,8 @@ public:
virtual std::optional<int64_t> MaxSatSize(bool use_max_sig) const { return {}; }
std::optional<int64_t> MaxSatisfactionWeight(bool) const override { return {}; }
+
+ std::optional<int64_t> MaxSatisfactionElems() const override { return {}; }
};
/** A parsed addr(A) descriptor. */
@@ -795,6 +797,8 @@ public:
std::optional<int64_t> MaxSatisfactionWeight(bool use_max_sig) const override {
return *MaxSatSize(use_max_sig) * WITNESS_SCALE_FACTOR;
}
+
+ std::optional<int64_t> MaxSatisfactionElems() const override { return 1; }
};
/** A parsed pkh(P) descriptor. */
@@ -822,6 +826,8 @@ public:
std::optional<int64_t> MaxSatisfactionWeight(bool use_max_sig) const override {
return *MaxSatSize(use_max_sig) * WITNESS_SCALE_FACTOR;
}
+
+ std::optional<int64_t> MaxSatisfactionElems() const override { return 2; }
};
/** A parsed wpkh(P) descriptor. */
@@ -849,6 +855,8 @@ public:
std::optional<int64_t> MaxSatisfactionWeight(bool use_max_sig) const override {
return MaxSatSize(use_max_sig);
}
+
+ std::optional<int64_t> MaxSatisfactionElems() const override { return 2; }
};
/** A parsed combo(P) descriptor. */
@@ -909,6 +917,8 @@ public:
std::optional<int64_t> MaxSatisfactionWeight(bool use_max_sig) const override {
return *MaxSatSize(use_max_sig) * WITNESS_SCALE_FACTOR;
}
+
+ std::optional<int64_t> MaxSatisfactionElems() const override { return 1 + m_threshold; }
};
/** A parsed (sorted)multi_a(...) descriptor. Always uses x-only pubkeys. */
@@ -943,6 +953,8 @@ public:
std::optional<int64_t> MaxSatSize(bool use_max_sig) const override {
return (1 + 65) * m_threshold + (m_pubkey_args.size() - m_threshold);
}
+
+ std::optional<int64_t> MaxSatisfactionElems() const override { return m_pubkey_args.size(); }
};
/** A parsed sh(...) descriptor. */
@@ -983,6 +995,11 @@ public:
}
return {};
}
+
+ std::optional<int64_t> MaxSatisfactionElems() const override {
+ if (const auto sub_elems = m_subdescriptor_args[0]->MaxSatisfactionElems()) return 1 + *sub_elems;
+ return {};
+ }
};
/** A parsed wsh(...) descriptor. */
@@ -1014,6 +1031,11 @@ public:
std::optional<int64_t> MaxSatisfactionWeight(bool use_max_sig) const override {
return MaxSatSize(use_max_sig);
}
+
+ std::optional<int64_t> MaxSatisfactionElems() const override {
+ if (const auto sub_elems = m_subdescriptor_args[0]->MaxSatisfactionElems()) return 1 + *sub_elems;
+ return {};
+ }
};
/** A parsed tr(...) descriptor. */
@@ -1074,6 +1096,11 @@ public:
// FIXME: We assume keypath spend, which can lead to very large underestimations.
return 1 + 65;
}
+
+ std::optional<int64_t> MaxSatisfactionElems() const override {
+ // FIXME: See above, we assume keypath spend.
+ return 1;
+ }
};
/* We instantiate Miniscript here with a simple integer as key type.
@@ -1164,6 +1191,10 @@ public:
// For Miniscript we always assume high-R ECDSA signatures.
return m_node->GetWitnessSize();
}
+
+ std::optional<int64_t> MaxSatisfactionElems() const override {
+ return m_node->GetStackSize();
+ }
};
/** A parsed rawtr(...) descriptor. */
@@ -1189,6 +1220,11 @@ public:
// We can't know whether there is a script path, so assume key path spend.
return 1 + 65;
}
+
+ std::optional<int64_t> MaxSatisfactionElems() const override {
+ // See above, we assume keypath spend.
+ return 1;
+ }
};
////////////////////////////////////////////////////////////////////////////
diff --git a/src/script/descriptor.h b/src/script/descriptor.h
index 32f01c9154..caa5d1608d 100644
--- a/src/script/descriptor.h
+++ b/src/script/descriptor.h
@@ -155,6 +155,9 @@ struct Descriptor {
* @param use_max_sig Whether to assume ECDSA signatures will have a high-r.
*/
virtual std::optional<int64_t> MaxSatisfactionWeight(bool use_max_sig) const = 0;
+
+ /** Get the maximum size number of stack elements for satisfying this descriptor. */
+ virtual std::optional<int64_t> MaxSatisfactionElems() const = 0;
};
/** Parse a `descriptor` string. Included private keys are put in `out`.
diff --git a/src/test/descriptor_tests.cpp b/src/test/descriptor_tests.cpp
index c2c3988f0d..3a30ef453e 100644
--- a/src/test/descriptor_tests.cpp
+++ b/src/test/descriptor_tests.cpp
@@ -154,7 +154,8 @@ void DoCheck(std::string prv, std::string pub, const std::string& norm_pub, int
const bool is_nontop_or_nonsolvable{!parse_priv->IsSolvable() || !parse_priv->GetOutputType()};
const auto max_sat_maxsig{parse_priv->MaxSatisfactionWeight(true)};
const auto max_sat_nonmaxsig{parse_priv->MaxSatisfactionWeight(true)};
- const bool is_input_size_info_set{max_sat_maxsig && max_sat_nonmaxsig};
+ const auto max_elems{parse_priv->MaxSatisfactionElems()};
+ const bool is_input_size_info_set{max_sat_maxsig && max_sat_nonmaxsig && max_elems};
BOOST_CHECK_MESSAGE(is_input_size_info_set || is_nontop_or_nonsolvable, prv);
// The ScriptSize() must match the size of the Script string. (ScriptSize() is set for all descs but 'combo()'.)
diff --git a/src/test/fuzz/descriptor_parse.cpp b/src/test/fuzz/descriptor_parse.cpp
index a866cdca9a..26c219d6c8 100644
--- a/src/test/fuzz/descriptor_parse.cpp
+++ b/src/test/fuzz/descriptor_parse.cpp
@@ -139,9 +139,10 @@ static void TestDescriptor(const Descriptor& desc, FlatSigningProvider& sig_prov
const auto max_sat_maxsig{desc.MaxSatisfactionWeight(true)};
const auto max_sat_nonmaxsig{desc.MaxSatisfactionWeight(true)};
+ const auto max_elems{desc.MaxSatisfactionElems()};
// We must be able to estimate the max satisfaction size for any solvable descriptor (but combo).
const bool is_nontop_or_nonsolvable{!is_solvable || !desc.GetOutputType()};
- const bool is_input_size_info_set{max_sat_maxsig && max_sat_nonmaxsig};
+ const bool is_input_size_info_set{max_sat_maxsig && max_sat_nonmaxsig && max_elems};
assert(is_input_size_info_set || is_nontop_or_nonsolvable);
}
diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index 81dd033177..750b6c100b 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -59,19 +59,20 @@ static std::optional<int64_t> MaxInputWeight(const Descriptor& desc, const std::
const CCoinControl* coin_control, const bool tx_is_segwit,
const bool can_grind_r) {
if (const auto sat_weight = desc.MaxSatisfactionWeight(!can_grind_r || UseMaxSig(txin, coin_control))) {
- const bool is_segwit = IsSegwit(desc);
- // Account for the size of the scriptsig and the number of elements on the witness stack. Note
- // that if any input in the transaction is spending a witness program, we need to specify the
- // witness stack size for every input regardless of whether it is segwit itself.
- // NOTE: this also works in case of mixed scriptsig-and-witness such as in p2sh-wrapped segwit v0
- // outputs. In this case the size of the scriptsig length will always be one (since the redeemScript
- // is always a push of the witness program in this case, which is smaller than 253 bytes).
- const int64_t scriptsig_len = is_segwit ? 1 : GetSizeOfCompactSize(*sat_weight / WITNESS_SCALE_FACTOR);
- // FIXME: use the real number of stack elements instead of the "1" placeholder.
- const int64_t witstack_len = is_segwit ? GetSizeOfCompactSize(1) : (tx_is_segwit ? 1 : 0);
- // previous txid + previous vout + sequence + scriptsig len + witstack size + scriptsig or witness
- // NOTE: sat_weight already accounts for the witness discount accordingly.
- return (32 + 4 + 4 + scriptsig_len) * WITNESS_SCALE_FACTOR + witstack_len + *sat_weight;
+ if (const auto elems_count = desc.MaxSatisfactionElems()) {
+ const bool is_segwit = IsSegwit(desc);
+ // Account for the size of the scriptsig and the number of elements on the witness stack. Note
+ // that if any input in the transaction is spending a witness program, we need to specify the
+ // witness stack size for every input regardless of whether it is segwit itself.
+ // NOTE: this also works in case of mixed scriptsig-and-witness such as in p2sh-wrapped segwit v0
+ // outputs. In this case the size of the scriptsig length will always be one (since the redeemScript
+ // is always a push of the witness program in this case, which is smaller than 253 bytes).
+ const int64_t scriptsig_len = is_segwit ? 1 : GetSizeOfCompactSize(*sat_weight / WITNESS_SCALE_FACTOR);
+ const int64_t witstack_len = is_segwit ? GetSizeOfCompactSize(*elems_count) : (tx_is_segwit ? 1 : 0);
+ // previous txid + previous vout + sequence + scriptsig len + witstack size + scriptsig or witness
+ // NOTE: sat_weight already accounts for the witness discount accordingly.
+ return (32 + 4 + 4 + scriptsig_len) * WITNESS_SCALE_FACTOR + witstack_len + *sat_weight;
+ }
}
return {};
diff --git a/src/wallet/test/walletload_tests.cpp b/src/wallet/test/walletload_tests.cpp
index 1c6f8c5cba..302db455b1 100644
--- a/src/wallet/test/walletload_tests.cpp
+++ b/src/wallet/test/walletload_tests.cpp
@@ -33,6 +33,7 @@ public:
void ExpandPrivate(int pos, const SigningProvider& provider, FlatSigningProvider& out) const override {}
std::optional<int64_t> ScriptSize() const override { return {}; }
std::optional<int64_t> MaxSatisfactionWeight(bool) const override { return {}; }
+ std::optional<int64_t> MaxSatisfactionElems() const override { return {}; }
};
BOOST_FIXTURE_TEST_CASE(wallet_load_descriptors, TestingSetup)