aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorW. J. van der Laan <laanwj@protonmail.com>2021-08-26 14:07:06 +0200
committerW. J. van der Laan <laanwj@protonmail.com>2021-08-26 14:26:01 +0200
commit4a25e39624e2d75bac2c65d50d824e6291513824 (patch)
tree3091f624b2a18108e4fd2529b22696bfc1417d00
parentd3bd5410f64e96d106486643e3d4300a9fb0ee71 (diff)
parent32e1424f84a30194dc5baa7108cf7d958ea0afcd (diff)
downloadbitcoin-4a25e39624e2d75bac2c65d50d824e6291513824.tar.xz
Merge bitcoin/bitcoin#22629: [22.x] rc3 backports
32e1424f84a30194dc5baa7108cf7d958ea0afcd Fix build with Boost 1.77.0 (Rafael Sadowski) cb34a0aafe21881b45a664432ad404a3dbf2f881 qt: Handle new added plurals in bitcoin_en.ts (Hennadii Stepanov) 068985c02e20b28c717a336e6b226e98060a9a45 doc: Mention the flat directory structure for uploads (Andrew Chow) 27d43e5bd40590d18b4cf88a4e98ddabbc93fb9a guix: Don't include directory name in SHA256SUMS (Andrew Chow) 88fb7e37ad37f2a262c7bf1f35ce77aa57113f32 test: fix bug in 22686 (S3RK) 63fec7e2958cd4349d73faf854e0665e7af30965 clientversion: No suffix #if CLIENT_VERSION_IS_RELEASE (Carl Dong) dfaffbeb6306be2e3bf447642f271c7fc733ae5e test: Test for ApproximateBestSubset edge case with too little fees (Andrew Chow) e86b023606193ca044f9ce20c88958d693585734 wallet: Assert that enough was selected to cover the fees (Andrew Chow) ffc81e2048bc9d3887211174b58f798b981f8c64 wallet: Use GetSelectionAmount for target value calculations (Andrew Chow) ce77b45a1f4c6ff5bb0a283ffdd0999e734c1fb0 release: Release with separate SHA256SUMS and sig files (Carl Dong) cb491bd5a717c280e23727a2ca3918d6ff6968b3 guix-verify: Non-zero exit code when anything fails (Carl Dong) 6a611d2e3c3ec703b2a034a3dc5422a6535c648b gui: ensure external signer option remains disabled without signers (Andrew Chow) e9b44876842df254fb4a2856702b74fe8c01ba6d qt: Fix regression in "Encrypt Wallet" menu item (Hennadii Stepanov) 57fce067a322d4b30ae8516795d5d2a1fe2f9f66 consensus/params: simplify ValidDeployment check to avoid gcc warning (Anthony Towns) e9d30fbb3a90dfafebdb026a53b4f632614d660e ci: Run fuzzer task for the master branch only (Hennadii Stepanov) Pull request description: Backported: 1) #22730 1) https://github.com/bitcoin-core/gui/pull/393 1) #22597 1) https://github.com/bitcoin-core/gui/pull/396 1) #22643 1) #22642 1) #22685 1) #22686 1) #22654 1) #22742 1) https://github.com/bitcoin-core/gui/pull/406 1) #22713 ACKs for top commit: laanwj: Code list-of-backported-PRs review ACK 32e1424f84a30194dc5baa7108cf7d958ea0afcd Tree-SHA512: f5e2dd1be6cdcd39368eeb5d297b3ff4418d0bf2e70c90e59ab4ba1dbf16f773045d877b4997511de58c3aca75a978dcf043e338bad23951557e2a27ccc845f6
-rw-r--r--.cirrus.yml1
-rwxr-xr-xcontrib/guix/guix-attest28
-rwxr-xr-xcontrib/guix/guix-verify6
-rw-r--r--doc/release-process.md36
-rw-r--r--src/clientversion.cpp6
-rw-r--r--src/consensus/params.h4
-rw-r--r--src/deploymentstatus.cpp17
-rw-r--r--src/fs.cpp4
-rw-r--r--src/qt/bitcoingui.cpp4
-rw-r--r--src/qt/createwalletdialog.cpp5
-rw-r--r--src/qt/createwalletdialog.h1
-rw-r--r--src/qt/locale/bitcoin_en.ts8
-rw-r--r--src/wallet/coinselection.cpp4
-rw-r--r--src/wallet/spend.cpp4
-rw-r--r--src/wallet/test/db_tests.cpp4
-rw-r--r--src/wallet/test/init_test_fixture.cpp4
-rwxr-xr-xtest/functional/rpc_fundrawtransaction.py57
17 files changed, 145 insertions, 48 deletions
diff --git a/.cirrus.yml b/.cirrus.yml
index 26bd27754f..16fd0cefc3 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -153,6 +153,7 @@ task:
task:
name: '[no depends, sanitizers: fuzzer,address,undefined,integer] [focal]'
+ only_if: $CIRRUS_BRANCH == $CIRRUS_DEFAULT_BRANCH || $CIRRUS_BASE_BRANCH == $CIRRUS_DEFAULT_BRANCH
<< : *GLOBAL_TASK_TEMPLATE
container:
image: ubuntu:focal
diff --git a/contrib/guix/guix-attest b/contrib/guix/guix-attest
index dcf709b542..6e12cbead7 100755
--- a/contrib/guix/guix-attest
+++ b/contrib/guix/guix-attest
@@ -159,23 +159,21 @@ Hint: You may wish to remove the existing attestations and their signatures by
EOF
}
-# Given a document with unix line endings (just <LF>) in stdin, make all lines
-# end in <CR><LF> and make sure there's no trailing <LF> at the end of the file.
-#
-# This is necessary as cleartext signatures are calculated on text after their
-# line endings are canonicalized.
+echo "Attesting to build outputs for version: '${VERSION}'"
+echo ""
+
+# Given a SHA256SUMS file as stdin that has lines like:
+# 0ba536819b221a91d3d42e978be016aac918f40984754d74058aa0c921cd3ea6 a/b/d/c/d/s/bitcoin-22.0rc2-riscv64-linux-gnu.tar.gz
+# ...
#
-# For more information:
-# 1. https://security.stackexchange.com/a/104261
-# 2. https://datatracker.ietf.org/doc/html/rfc4880#section-7.1
+# Replace each line's file name with its basename:
+# 0ba536819b221a91d3d42e978be016aac918f40984754d74058aa0c921cd3ea6 bitcoin-22.0rc2-riscv64-linux-gnu.tar.gz
+# ...
#
-rfc4880_normalize_document() {
- sed 's/$/\r/' | head -c -2
+basenameify_SHA256SUMS() {
+ sed -E 's@(^[[:xdigit:]]{64}[[:space:]]+).+/([^/]+$)@\1\2@'
}
-echo "Attesting to build outputs for version: '${VERSION}'"
-echo ""
-
outsigdir="$GUIX_SIGS_REPO/$VERSION/$signer_name"
mkdir -p "$outsigdir"
(
@@ -188,7 +186,7 @@ mkdir -p "$outsigdir"
cat "${noncodesigned_fragments[@]}" \
| sort -u \
| sort -k2 \
- | rfc4880_normalize_document \
+ | basenameify_SHA256SUMS \
> "$temp_noncodesigned"
if [ -e noncodesigned.SHA256SUMS ]; then
# The SHA256SUMS already exists, make sure it's exactly what we
@@ -216,7 +214,7 @@ mkdir -p "$outsigdir"
cat "${sha256sum_fragments[@]}" \
| sort -u \
| sort -k2 \
- | rfc4880_normalize_document \
+ | basenameify_SHA256SUMS \
> "$temp_all"
if [ -e all.SHA256SUMS ]; then
# The SHA256SUMS already exists, make sure it's exactly what we
diff --git a/contrib/guix/guix-verify b/contrib/guix/guix-verify
index e4863f115b..02ae022741 100755
--- a/contrib/guix/guix-verify
+++ b/contrib/guix/guix-verify
@@ -77,11 +77,13 @@ verify() {
echo ""
echo "Hint: Either the signature is invalid or the public key is missing"
echo ""
+ failure=1
elif ! diff --report-identical "$compare_manifest" "$current_manifest" 1>&2; then
echo "ERR: The SHA256SUMS attestation in these two directories differ:"
echo " '${compare_manifest}'"
echo " '${current_manifest}'"
echo ""
+ failure=1
else
echo "Verified: '${current_manifest}'"
echo ""
@@ -166,3 +168,7 @@ if (( ${#all_noncodesigned[@]} + ${#all_all[@]} == 0 )); then
echo ""
exit 1
fi
+
+if [ -n "$failure" ]; then
+ exit 1
+fi
diff --git a/doc/release-process.md b/doc/release-process.md
index 26ac259c46..b626271814 100644
--- a/doc/release-process.md
+++ b/doc/release-process.md
@@ -199,30 +199,22 @@ popd
### After 3 or more people have guix-built and their results match:
-Combine `all.SHA256SUMS` and `all.SHA256SUMS.asc` into a clear-signed
-`SHA256SUMS.asc` message:
-
-```sh
-echo -e "-----BEGIN PGP SIGNED MESSAGE-----\nHash: SHA256\n\n$(cat all.SHA256SUMS)\n$(cat filename.txt.asc)" > SHA256SUMS.asc
-```
-
-Here's an equivalent, more readable command if you're confident that you won't
-mess up whitespaces when copy-pasting:
+Combine the `all.SHA256SUMS.asc` file from all signers into `SHA256SUMS.asc`:
```bash
-cat << EOF > SHA256SUMS.asc
------BEGIN PGP SIGNED MESSAGE-----
-Hash: SHA256
-
-$(cat all.SHA256SUMS)
-$(cat all.SHA256SUMS.asc)
-EOF
+cat "$VERSION"/*/all.SHA256SUMS.asc > SHA256SUMS.asc
```
-- Upload to the bitcoincore.org server (`/var/www/bin/bitcoin-core-${VERSION}`):
- 1. The contents of `./bitcoin/guix-build-${VERSION}/output`, except for
+
+- Upload to the bitcoincore.org server (`/var/www/bin/bitcoin-core-${VERSION}/`):
+ 1. The contents of each `./bitcoin/guix-build-${VERSION}/output/${HOST}/` directory, except for
`*-debug*` files.
+ Guix will output all of the results into host subdirectories, but the SHA256SUMS
+ file does not include these subdirectories. In order for downloads via torrent
+ to verify without directory structure modification, all of the uploaded files
+ need to be in the same directory as the SHA256SUMS file.
+
The `*-debug*` files generated by the guix build contain debug symbols
for troubleshooting by developers. It is assumed that anyone that is
interested in debugging can run guix to generate the files for
@@ -230,7 +222,13 @@ EOF
as save storage space *do not upload these to the bitcoincore.org server,
nor put them in the torrent*.
- 2. The combined clear-signed message you just created `SHA256SUMS.asc`
+ ```sh
+ find guix-build-${VERSION}/output/ -maxdepth 2 -type f -not -name "SHA256SUMS.part" -and -not -name "*debug*" -exec scp {} user@bitcoincore.org:/var/www/bin/bitcoin-core-${VERSION} \;
+ ```
+
+ 2. The `SHA256SUMS` file
+
+ 3. The `SHA256SUMS.asc` combined signature file you just created
- Create a torrent of the `/var/www/bin/bitcoin-core-${VERSION}` directory such
that at the top level there is only one file: the `bitcoin-core-${VERSION}`
diff --git a/src/clientversion.cpp b/src/clientversion.cpp
index 29c38e2d3b..195f58b3f3 100644
--- a/src/clientversion.cpp
+++ b/src/clientversion.cpp
@@ -30,8 +30,10 @@ const std::string CLIENT_NAME("Satoshi");
#define BUILD_DESC BUILD_GIT_TAG
#define BUILD_SUFFIX ""
#else
- #define BUILD_DESC "v" STRINGIZE(CLIENT_VERSION_MAJOR) "." STRINGIZE(CLIENT_VERSION_MINOR) "." STRINGIZE(CLIENT_VERSION_BUILD)
- #ifdef BUILD_GIT_COMMIT
+ #define BUILD_DESC "v" PACKAGE_VERSION
+ #if CLIENT_VERSION_IS_RELEASE
+ #define BUILD_SUFFIX ""
+ #elif defined(BUILD_GIT_COMMIT)
#define BUILD_SUFFIX "-" BUILD_GIT_COMMIT
#elif defined(GIT_COMMIT_ID)
#define BUILD_SUFFIX "-g" GIT_COMMIT_ID
diff --git a/src/consensus/params.h b/src/consensus/params.h
index 9205cfee87..77bf7fd0d8 100644
--- a/src/consensus/params.h
+++ b/src/consensus/params.h
@@ -23,7 +23,7 @@ enum BuriedDeployment : int16_t {
DEPLOYMENT_CSV,
DEPLOYMENT_SEGWIT,
};
-constexpr bool ValidDeployment(BuriedDeployment dep) { return DEPLOYMENT_HEIGHTINCB <= dep && dep <= DEPLOYMENT_SEGWIT; }
+constexpr bool ValidDeployment(BuriedDeployment dep) { return dep <= DEPLOYMENT_SEGWIT; }
enum DeploymentPos : uint16_t {
DEPLOYMENT_TESTDUMMY,
@@ -31,7 +31,7 @@ enum DeploymentPos : uint16_t {
// NOTE: Also add new deployments to VersionBitsDeploymentInfo in deploymentinfo.cpp
MAX_VERSION_BITS_DEPLOYMENTS
};
-constexpr bool ValidDeployment(DeploymentPos dep) { return DEPLOYMENT_TESTDUMMY <= dep && dep <= DEPLOYMENT_TAPROOT; }
+constexpr bool ValidDeployment(DeploymentPos dep) { return dep < MAX_VERSION_BITS_DEPLOYMENTS; }
/**
* Struct for each individual consensus rule change using BIP9.
diff --git a/src/deploymentstatus.cpp b/src/deploymentstatus.cpp
index 9007800421..bba86639a3 100644
--- a/src/deploymentstatus.cpp
+++ b/src/deploymentstatus.cpp
@@ -7,6 +7,8 @@
#include <consensus/params.h>
#include <versionbits.h>
+#include <type_traits>
+
VersionBitsCache g_versionbitscache;
/* Basic sanity checking for BuriedDeployment/DeploymentPos enums and
@@ -15,3 +17,18 @@ VersionBitsCache g_versionbitscache;
static_assert(ValidDeployment(Consensus::DEPLOYMENT_TESTDUMMY), "sanity check of DeploymentPos failed (TESTDUMMY not valid)");
static_assert(!ValidDeployment(Consensus::MAX_VERSION_BITS_DEPLOYMENTS), "sanity check of DeploymentPos failed (MAX value considered valid)");
static_assert(!ValidDeployment(static_cast<Consensus::BuriedDeployment>(Consensus::DEPLOYMENT_TESTDUMMY)), "sanity check of BuriedDeployment failed (overlaps with DeploymentPos)");
+
+/* ValidDeployment only checks upper bounds for ensuring validity.
+ * This checks that the lowest possible value or the type is also a
+ * (specific) valid deployment so that lower bounds don't need to be checked.
+ */
+
+template<typename T, T x>
+static constexpr bool is_minimum()
+{
+ using U = typename std::underlying_type<T>::type;
+ return x == std::numeric_limits<U>::min();
+}
+
+static_assert(is_minimum<Consensus::BuriedDeployment, Consensus::DEPLOYMENT_HEIGHTINCB>(), "heightincb is not minimum value for BuriedDeployment");
+static_assert(is_minimum<Consensus::DeploymentPos, Consensus::DEPLOYMENT_TESTDUMMY>(), "testdummy is not minimum value for DeploymentPos");
diff --git a/src/fs.cpp b/src/fs.cpp
index 4f20ca4d28..89c7ad27dc 100644
--- a/src/fs.cpp
+++ b/src/fs.cpp
@@ -242,7 +242,11 @@ void ofstream::close()
}
#else // __GLIBCXX__
+#if BOOST_VERSION >= 107700
+static_assert(sizeof(*BOOST_FILESYSTEM_C_STR(fs::path())) == sizeof(wchar_t),
+#else
static_assert(sizeof(*fs::path().BOOST_FILESYSTEM_C_STR) == sizeof(wchar_t),
+#endif // BOOST_VERSION >= 107700
"Warning: This build is using boost::filesystem ofstream and ifstream "
"implementations which will fail to open paths containing multibyte "
"characters. You should delete this static_assert to ignore this warning, "
diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp
index f8aeb01659..863225099a 100644
--- a/src/qt/bitcoingui.cpp
+++ b/src/qt/bitcoingui.cpp
@@ -682,8 +682,6 @@ void BitcoinGUI::addWallet(WalletModel* walletModel)
m_wallet_selector_label_action->setVisible(true);
m_wallet_selector_action->setVisible(true);
}
- const QString display_name = walletModel->getDisplayName();
- m_wallet_selector->addItem(display_name, QVariant::fromValue(walletModel));
connect(wallet_view, &WalletView::outOfSyncWarningClicked, this, &BitcoinGUI::showModalOverlay);
connect(wallet_view, &WalletView::transactionClicked, this, &BitcoinGUI::gotoHistoryPage);
@@ -696,6 +694,8 @@ void BitcoinGUI::addWallet(WalletModel* walletModel)
connect(wallet_view, &WalletView::hdEnabledStatusChanged, this, &BitcoinGUI::updateWalletStatus);
connect(this, &BitcoinGUI::setPrivacy, wallet_view, &WalletView::setPrivacy);
wallet_view->setPrivacy(isPrivacyModeActivated());
+ const QString display_name = walletModel->getDisplayName();
+ m_wallet_selector->addItem(display_name, QVariant::fromValue(walletModel));
}
void BitcoinGUI::removeWallet(WalletModel* walletModel)
diff --git a/src/qt/createwalletdialog.cpp b/src/qt/createwalletdialog.cpp
index dc24bbc6a6..f9a61c3e60 100644
--- a/src/qt/createwalletdialog.cpp
+++ b/src/qt/createwalletdialog.cpp
@@ -32,7 +32,7 @@ CreateWalletDialog::CreateWalletDialog(QWidget* parent) :
// set to true, enable it when isEncryptWalletChecked is false.
ui->disable_privkeys_checkbox->setEnabled(!checked);
#ifdef ENABLE_EXTERNAL_SIGNER
- ui->external_signer_checkbox->setEnabled(!checked);
+ ui->external_signer_checkbox->setEnabled(m_has_signers && !checked);
#endif
// When the disable_privkeys_checkbox is disabled, uncheck it.
if (!ui->disable_privkeys_checkbox->isEnabled()) {
@@ -115,7 +115,8 @@ CreateWalletDialog::~CreateWalletDialog()
void CreateWalletDialog::setSigners(const std::vector<ExternalSigner>& signers)
{
- if (!signers.empty()) {
+ m_has_signers = !signers.empty();
+ if (m_has_signers) {
ui->external_signer_checkbox->setEnabled(true);
ui->external_signer_checkbox->setChecked(true);
ui->encrypt_wallet_checkbox->setEnabled(false);
diff --git a/src/qt/createwalletdialog.h b/src/qt/createwalletdialog.h
index 25ddf97585..fc13cc44eb 100644
--- a/src/qt/createwalletdialog.h
+++ b/src/qt/createwalletdialog.h
@@ -35,6 +35,7 @@ public:
private:
Ui::CreateWalletDialog *ui;
+ bool m_has_signers = false;
};
#endif // BITCOIN_QT_CREATEWALLETDIALOG_H
diff --git a/src/qt/locale/bitcoin_en.ts b/src/qt/locale/bitcoin_en.ts
index 7026f49c01..47c002498a 100644
--- a/src/qt/locale/bitcoin_en.ts
+++ b/src/qt/locale/bitcoin_en.ts
@@ -749,8 +749,8 @@ Signing is only possible with addresses of the type &apos;legacy&apos;.</source>
<source>%n active connection(s) to Bitcoin network.</source>
<extracomment>A substring of the tooltip.</extracomment>
<translation type="unfinished">
- <numerusform></numerusform>
- <numerusform></numerusform>
+ <numerusform>%n active connection to Bitcoin network.</numerusform>
+ <numerusform>%n active connections to Bitcoin network.</numerusform>
</translation>
</message>
<message>
@@ -1376,8 +1376,8 @@ Signing is only possible with addresses of the type &apos;legacy&apos;.</source>
<source>(sufficient to restore backups %n day(s) old)</source>
<extracomment>Explanatory text on the capability of the current prune target.</extracomment>
<translation type="unfinished">
- <numerusform></numerusform>
- <numerusform></numerusform>
+ <numerusform>(sufficient to restore backups %n day old)</numerusform>
+ <numerusform>(sufficient to restore backups %n days old)</numerusform>
</translation>
</message>
<message>
diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp
index 6d502e1df1..25b1ee07e4 100644
--- a/src/wallet/coinselection.cpp
+++ b/src/wallet/coinselection.cpp
@@ -195,7 +195,7 @@ static void ApproximateBestSubset(const std::vector<OutputGroup>& groups, const
//the selection random.
if (nPass == 0 ? insecure_rand.randbool() : !vfIncluded[i])
{
- nTotal += groups[i].m_value;
+ nTotal += groups[i].GetSelectionAmount();
vfIncluded[i] = true;
if (nTotal >= nTargetValue)
{
@@ -205,7 +205,7 @@ static void ApproximateBestSubset(const std::vector<OutputGroup>& groups, const
nBest = nTotal;
vfBest = vfIncluded;
}
- nTotal -= groups[i].m_value;
+ nTotal -= groups[i].GetSelectionAmount();
vfIncluded[i] = false;
}
}
diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index 6a8df437ae..e403b69b49 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -778,6 +778,10 @@ bool CWallet::CreateTransactionInternal(
fee_needed = coin_selection_params.m_effective_feerate.GetFee(nBytes);
}
+ // The only time that fee_needed should be less than the amount available for fees (in change_and_fee - change_amount) is when
+ // we are subtracting the fee from the outputs. If this occurs at any other time, it is a bug.
+ assert(coin_selection_params.m_subtract_fee_outputs || fee_needed <= change_and_fee - change_amount);
+
// Update nFeeRet in case fee_needed changed due to dropping the change output
if (fee_needed <= change_and_fee - change_amount) {
nFeeRet = change_and_fee - change_amount;
diff --git a/src/wallet/test/db_tests.cpp b/src/wallet/test/db_tests.cpp
index 17f5264b45..16cb7e0baf 100644
--- a/src/wallet/test/db_tests.cpp
+++ b/src/wallet/test/db_tests.cpp
@@ -25,7 +25,11 @@ BOOST_AUTO_TEST_CASE(getwalletenv_file)
std::string test_name = "test_name.dat";
const fs::path datadir = gArgs.GetDataDirNet();
fs::path file_path = datadir / test_name;
+#if BOOST_VERSION >= 107700
+ std::ofstream f(BOOST_FILESYSTEM_C_STR(file_path));
+#else
std::ofstream f(file_path.BOOST_FILESYSTEM_C_STR);
+#endif // BOOST_VERSION >= 107700
f.close();
std::string filename;
diff --git a/src/wallet/test/init_test_fixture.cpp b/src/wallet/test/init_test_fixture.cpp
index dd9354848d..53c972c46d 100644
--- a/src/wallet/test/init_test_fixture.cpp
+++ b/src/wallet/test/init_test_fixture.cpp
@@ -32,7 +32,11 @@ InitWalletDirTestingSetup::InitWalletDirTestingSetup(const std::string& chainNam
fs::create_directories(m_walletdir_path_cases["default"]);
fs::create_directories(m_walletdir_path_cases["custom"]);
fs::create_directories(m_walletdir_path_cases["relative"]);
+#if BOOST_VERSION >= 107700
+ std::ofstream f(BOOST_FILESYSTEM_C_STR(m_walletdir_path_cases["file"]));
+#else
std::ofstream f(m_walletdir_path_cases["file"].BOOST_FILESYSTEM_C_STR);
+#endif // BOOST_VERSION >= 107700
f.close();
}
diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py
index fa98c44152..fcc310394a 100755
--- a/test/functional/rpc_fundrawtransaction.py
+++ b/test/functional/rpc_fundrawtransaction.py
@@ -99,6 +99,7 @@ class RawTransactionsTest(BitcoinTestFramework):
self.test_subtract_fee_with_presets()
self.test_transaction_too_large()
self.test_include_unsafe()
+ self.test_22670()
def test_change_position(self):
"""Ensure setting changePosition in fundraw with an exact match is handled properly."""
@@ -969,6 +970,62 @@ class RawTransactionsTest(BitcoinTestFramework):
signedtx = wallet.signrawtransactionwithwallet(fundedtx['hex'])
wallet.sendrawtransaction(signedtx['hex'])
+ def test_22670(self):
+ # In issue #22670, it was observed that ApproximateBestSubset may
+ # choose enough value to cover the target amount but not enough to cover the transaction fees.
+ # This leads to a transaction whose actual transaction feerate is lower than expected.
+ # However at normal feerates, the difference between the effective value and the real value
+ # that this bug is not detected because the transaction fee must be at least 0.01 BTC (the minimum change value).
+ # Otherwise the targeted minimum change value will be enough to cover the transaction fees that were not
+ # being accounted for. So the minimum relay fee is set to 0.1 BTC/kvB in this test.
+ self.log.info("Test issue 22670 ApproximateBestSubset bug")
+ # Make sure the default wallet will not be loaded when restarted with a high minrelaytxfee
+ self.nodes[0].unloadwallet(self.default_wallet_name, False)
+ feerate = Decimal("0.1")
+ self.restart_node(0, [f"-minrelaytxfee={feerate}", "-discardfee=0"]) # Set high minrelayfee, set discardfee to 0 for easier calculation
+
+ self.nodes[0].loadwallet(self.default_wallet_name, True)
+ funds = self.nodes[0].get_wallet_rpc(self.default_wallet_name)
+ self.nodes[0].createwallet(wallet_name="tester")
+ tester = self.nodes[0].get_wallet_rpc("tester")
+
+ # Because this test is specifically for ApproximateBestSubset, the target value must be greater
+ # than any single input available, and require more than 1 input. So we make 3 outputs
+ for i in range(0, 3):
+ funds.sendtoaddress(tester.getnewaddress(address_type="bech32"), 1)
+ self.nodes[0].generate(1)
+
+ # Create transactions in order to calculate fees for the target bounds that can trigger this bug
+ change_tx = tester.fundrawtransaction(tester.createrawtransaction([], [{funds.getnewaddress(): 1.5}]))
+ tx = tester.createrawtransaction([], [{funds.getnewaddress(): 2}])
+ no_change_tx = tester.fundrawtransaction(tx, {"subtractFeeFromOutputs": [0]})
+
+ overhead_fees = feerate * len(tx) / 2 / 1000
+ cost_of_change = change_tx["fee"] - no_change_tx["fee"]
+ fees = no_change_tx["fee"]
+ assert_greater_than(fees, 0.01)
+
+ def do_fund_send(target):
+ create_tx = tester.createrawtransaction([], [{funds.getnewaddress(): target}])
+ funded_tx = tester.fundrawtransaction(create_tx)
+ signed_tx = tester.signrawtransactionwithwallet(funded_tx["hex"])
+ assert signed_tx["complete"]
+ decoded_tx = tester.decoderawtransaction(signed_tx["hex"])
+ assert_equal(len(decoded_tx["vin"]), 3)
+ assert tester.testmempoolaccept([signed_tx["hex"]])[0]["allowed"]
+
+ # We want to choose more value than is available in 2 inputs when considering the fee,
+ # but not enough to need 3 inputs when not considering the fee.
+ # So the target value must be at least 2.00000001 - fee.
+ lower_bound = Decimal("2.00000001") - fees
+ # The target value must be at most 2 - cost_of_change - not_input_fees - min_change (these are all
+ # included in the target before ApproximateBestSubset).
+ upper_bound = Decimal("2.0") - cost_of_change - overhead_fees - Decimal("0.01")
+ assert_greater_than_or_equal(upper_bound, lower_bound)
+ do_fund_send(lower_bound)
+ do_fund_send(upper_bound)
+
+ self.restart_node(0)
if __name__ == '__main__':
RawTransactionsTest().main()