[pull] master from bitcoin:master - #1877
Merged
Merged
Conversation
importprunedfunds only accepted transactions that pay the wallet: it checked IsMine and added the transaction directly, so a transaction that only spends the wallet's outputs was rejected (#21647). Route the import through AddToWalletIfInvolvingMe, which already checks both IsMine and IsFromMe, so spending transactions are accepted without duplicating the involvement check in the RPC. Fixes #21647
The assumed blockchain size is maintained in GiB, but the first-run warning labels it as GB. For pruned nodes, the disk-space check uses the lower prune target while the warning displays the full-chain size. Label the value as GiB and round the checked byte estimate up to GiB so the warning describes the selected storage mode. Co-authored-by: Luke Dashjr <luke-jr+git@utopios.org> Co-authored-by: Ava Chow <github@achow101.com>
Pure refactor with no behaviour change. Moving the single-shot parse into a named helper lets later commits add checks after it without its early returns skipping them, and lets the framing invariant from #35759 apply to requests parsed by other means.
The input was capped at 4096 bytes, which is below the 8192-byte MAX_HEADERS_SIZE enforced in HTTPHeaders::Read(), so no input this target generates could reach the headers size limit. The cap dates back to the original libevent harness and predates the parser the target now exercises.
The target parses each input in one pass: a single LineReader over the whole buffer, each Load* called once. A request that arrives over several I/O cycles takes a different path, which resumes where it left off, and the target could not reach it. Feed the same bytes through a client a slice at a time, so parsing has to resume across cycle boundaries, and check what must hold at each boundary. A completed request is always handed back, parsing only advances or fails, chunk progress never exceeds the declared chunk size, a failed request consumes nothing further, and nothing is parsed or consumed while a request is out with a worker.
Framing is defined by the byte stream, so how TCP happened to split it must not change what the parser produces. Run the same input twice, once delivered whole and once in arbitrary slices, and require both runs to agree on the requests parsed, their order, and whether parsing failed. The leftover receive buffer is only compared when neither run failed: the whole buffer is discarded on a parse error, so how much had arrived by then legitimately differs between the two runs.
SIGHASH_SINGLE only commits to the output at the input's index. If the output at such position doesn't exist, it commits to no output at all (legacy uses a fixed sighash of 1, segwit v0 zeroes hashOutputs), which means the signature stays valid even when outputs are swapped, which is a footgun that lets funds be redirected without the owner's consent. SignTransaction() already skipped these inputs, but SignPSBTInput() did not, so walletprocesspsbt signed them. This moves the check into the CreateSig so both paths, and any future one, skip producing the detached signature.
-BEGIN VERIFY SCRIPT- git grep -l "m_default_max_tx_fee" src | xargs sed -i "s/m_default_max_tx_fee/m_max_tx_fee/g" git grep -l "getDefaultMaxTxFee" src | xargs sed -i "s/getDefaultMaxTxFee/getMaxTxFee/g" -END VERIFY SCRIPT- - The value is not always the default but can be configured during startup so `m_max_tx_fee` is the right name not `m_default_max_tx_fee`.
- Also update `-maxapsfee` option from `max_fee` to `max_aps_fee`. This fixes the ambiguity in the variable name. The comment on m_max_tx_fee described it as the value used "by default" for the wallet, but it is not always the default: it can be overridden via -maxtxfee. Drop "by default" to avoid the misleading wording.
- The commits adds a new wallet startup option `-maxfeerate` - The value will be used as the upper limit of wallet transaction fee rate. - The commit updates all instances where `-maxtxfee` value is utilized to check wallet transactions fee rate upper limit to now use `-maxfeerate` value.
`BroadcastTransaction` now accepts a fee rate limit as an additional input parameter. It rejects transactions whose fee exceeds the maximum fee amount permitted by that fee rate for the transaction vsize. Compare against `maxfeerate.GetFee(vsize)` instead of reconstructing a fee rate from the accepted fee. This preserves the existing rounded fee cap semantics used by `testmempoolaccept` and avoids rejecting transactions whose rounded fee is still within the configured limit. This allows callers to distinguish between failures caused by the absolute fee limit (`-maxtxfee`) and the fee rate limit (`maxfeerate` or `-maxfeerate`). `TestSimpleSpend` creates transactions paying `DEFAULT_TRANSACTION_MAXFEE`. Since `BroadcastTransaction` now also checks the fee rate limit, these small high-fee transactions would exceed `DEFAULT_MAX_TRANSACTION_FEERATE`. Update the test broadcast limit to be slightly above the transaction fee rate. In `wallet_fundrawtransaction.py`'s `test_22670` subtest, restart node 0 with `-maxfeerate` above `-minrelaytxfee` so high-feerate transactions can be broadcast.
- This distinguishes maxfeerate and maxtxfee error messages.
This commit prevents the wallet from submitting or broadcasting transactions above `-maxfeerate`, and tests the new functionality. Compare wallet transaction fees against `maxfeerate.GetFee(vsize)` instead of reconstructing a fee rate from the rounded fee. This allows transactions whose requested fee rate is at the configured limit, even when the resulting fee is rounded up to whole satoshis. `rpc_psbt.py` functional test nodes are modified to start with a custom `-maxfeerate=1`, because the test requires creating and broadcasting a transaction with a high fee rate. A node in some subtests in `wallet_fundrawtransaction.py` is restarted with `-maxfeerate=1` because those subtests require the node's wallet to be able to create and broadcast transactions with a high fee rate.
- Wallets cannot create transactions with fee rates below `-minrelaytxfee`, and they also reject transactions whose total fee exceeds `-maxtxfee`. - Warn when a 1 kvB transaction paying exactly `-maxtxfee` would still have a fee rate below `-minrelaytxfee`. In this configuration, some transactions that meet the minimum relay fee rate may exceed `-maxtxfee`, preventing the wallet from creating them. - Keep this warning independent from the existing high `-maxtxfee` warning, so users see both warnings when both conditions apply.
42215b8 init: correct first-run disk space estimate (Lőrinc) Pull request description: **Problem:** The assumed blockchain size is maintained in GiB, but the first-run disk-space warning labels it as GB. For pruned nodes, the warning also displays the full-chain estimate even though the check uses the lower of the prune target and that estimate. The chainparams API comments likewise describe both assumed sizes as GB. **Fix:** Label the warning as GiB, display the same rounded-up GiB estimate that the disk-space check uses, and document both assumed sizes as GiB. This PR revives the first-run warning fixes from #29678. ACKs for top commit: jeanpablojp: tACK 42215b8 achow101: ACK 42215b8 murchandamus: ACK 42215b8 Tree-SHA512: fa8a0e1eeefd674a4344772fa5e8f63fc3af9c106057c34fef47817d41cf162ad21f0e06412ad5bebc1504c57bb720e1c0ca5392e6f3d4052d3f8c7c360bac69
412540e i2p: update leaseset encryption types (jpk68) Pull request description: Updates the I2P SAM leaseset parameters to `6,4` (MLKEM-768/ECIES-X25519), as is [now recommended](https://i2p.net/en/docs/api/samv3/#signature-and-encryption-types) by the I2P project. ElGamal leasesets are now considered "legacy" and likely should not be used anymore. ACKs for top commit: kevkevinpal: ACK [412540e](412540e) davidgumberg: ACK 412540e achow101: ACK 412540e jonatack: ACK 412540e janb84: ACK 412540e Tree-SHA512: 639f6c58700f0a6ec471652d31b62d36ee16226afc13267a2064c199fd7e6102d7ac1b146fecaaee611235f8463fb1572318d21aa774c0a7e06ea4be06062a8c
fca8fef fuzz: assert HTTP framing is independent of stream segmentation (frankomosh) c368de3 fuzz: exercise the HTTPRequest state machine (frankomosh) d23b53e fuzz: allow http_request inputs to reach MAX_HEADERS_SIZE (frankomosh) 56af5e0 fuzz: extract http_request parse and framing check into helpers (frankomosh) Pull request description: Follow-up to #35735, adding a test to the state machine added there. As it is, the target parses the whole input in one pass, so the paths that resume a partly-read request are unreachable from it. This target now parses each input twice, once whole and once in random slices with a parse attempt after each, and requires both runs to produce the same request. HTTP framing is defined by the byte stream, so how it was split must not change the result. Also raises the input cap from 4096, which sat below MAX_HEADERS_SIZE and kept the header size limit out of reach. ACKs for top commit: jeanpablojp: tACK fca8fef hodlinator: ACK fca8fef pinheadmz: ACK fca8fef Tree-SHA512: 705fd05894c43de067365543f6e67387cf19e22f52191b4ede75ab9d8043c545ecdda21fc05097ac228378d27afacb9afad65e89e433cf8a7355d91b82a080c8
…ponding output 8df006f wallet: skip signing SIGHASH_SINGLE inputs with no corresponding output (furszy) Pull request description: `SIGHASH_SINGLE` only commits to the output at the input's index. If the output at such position doesn't exist, it commits to no output at all (legacy uses a fixed sighash of 1, segwit v0 zeroes `hashOutputs`), which means the signature stays valid even when outputs are swapped, which is a footgun that lets funds be redirected without the owner's consent. `SignTransaction()` already skipped these inputs, but `SignPSBTInput()` did not, so `walletprocesspsbt` signed them. This moves the check into the `CreateSig` so both paths, and any future one, skip producing the detached signature. Note: if there is a valid use for the segwit v0 case, I would rather re-allow it through an explicit opt-in arg than by default, so it is always a deliberate choice. Fixes #35977 ACKs for top commit: thomasbuilds: ACK 8df006f achow101: ACK 8df006f Tree-SHA512: ac1c9659910eae889475c65b8dde68e711c4d85d0d520af06411728591e573bf027b41a9ac6dd0f6e93de425841ac6c7d9e6c9f07936b9d8ae390941fbe6f1f1
f69e56b [doc]: add release notes (ismaelsadeeq) 40ab7aa [wallet]: warn when `-maxtxfee` conflicts with `-minrelaytxfee` (ismaelsadeeq) df1feeb [wallet]: enforce `-maxfeerate` on wallet transactions (ismaelsadeeq) d2c832e [util]: add a new transaction error type (ismaelsadeeq) 633c08d [node]: update `BroadcastTransaction` to check fee rate limit (ismaelsadeeq) 7bfce65 [wallet]: add `maxfeerate` wallet startup option (ismaelsadeeq) 607568f [wallet]: update `max_fee` to `max_tx_fee` (ismaelsadeeq) 6b913b6 doc: add missing verb to make sentence readable (ismaelsadeeq) a9c3969 scripted-diff: rename `m_default_max_tx_fee` to `m_max_tx_fee` (ismaelsadeeq) Pull request description: This PR fixes #29220 - The PR adds a wallet `-maxfeerate` startup option, as the upper limit of wallet transactions fee rate. - This fixes the ambiguity of using `maxtxfee` value to check the upper limit of transactions fee rate. - Wallet will not create a transaction with fee rate above `maxfeerate` value. - This PR adds a functional test that ensure the behavior is enforced. ACKs for top commit: achow101: ACK f69e56b polespinasa: ACK f69e56b Tree-SHA512: 595f451a2fd49ca887705e16ca82373c840c610d079de893bb67fba06e8b5c78646fb723255f226fdae7a6ad57a6f2b42e5d54b07d320c5c2b3a17c943add4ab
c2da52d wallet: allow importprunedfunds for spending transactions (8144225309) Pull request description: `importprunedfunds` only allowed importing transactions that credit the wallet (checked via `IsMine`), rejecting transactions that spend from it. This could leave a pruned wallet showing an incorrect balance: if a spending transaction was removed with `removeprunedfunds`, it couldn't be re-imported and would fail with "No addresses in wallet correspond to included transaction". This routes the import through `AddToWalletIfInvolvingMe()`, which already checks both `IsMine()` and `IsFromMe()`, so spending transactions are accepted and the involvement check is no longer duplicated in `importprunedfunds`. A functional test imports a transaction that spends from the wallet but has no outputs to it (entire UTXO sent externally). Fixes #21647 ACKs for top commit: achow101: ACK c2da52d Bicaru20: re-ACK c2da52d Tree-SHA512: 0ece38c75d3608948f1c29c5fca29b6420197b0a153e361decf232fd27edd5dd1f2e5d6f9625eee6f7691abd9992f95ef89c904d486ee927d03c88475e479b9c
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )