aboutsummaryrefslogtreecommitdiff
path: root/src/rpc
AgeCommit message (Collapse)Author
2024-09-11assumeUTXO: fix peers disconnection during syncfurszy
Because AssumeUTXO nodes prioritize tip synchronization, they relay their local address through the network before completing the background chain sync. This, combined with the advertising of full-node service (NODE_NETWORK), can result in an honest peer in IBD connecting to the AssumeUTXO node (while syncing) and requesting an historical block the node does not have. This behavior leads to an abrupt disconnection due to perceived unresponsiveness (lack of response) from the AssumeUTXO node. This lack of response occurs because nodes ignore getdata requests when they do not have the block data available (further discussion can be found in PR 30385). Fix this by refraining from signaling full-node service support while the background chain is being synced. During this period, the node will only signal 'NODE_NETWORK_LIMITED' support. Then, full-node ('NODE_NETWORK') support will be re-enabled once the background chain sync is completed. Github-Pull: bitcoin/bitcoin#30807 Rebased-From: 6d5812e5c852c233bd7ead2ceef051f8567619ed
2024-08-13Deduplicate list of chain strings in RPC help textsMartin Saposnic
2024-08-12doc: add missing "testnet4" network string in RPC/init help textsSebastian Falbesoner
The following bitcoind parameters / RPC calls missed the "testnet4" network string: - `-chain=` parameter - `getblockchaininfo` RPC, "chain" result - `getmininginfo` RPC, "chain" result
2024-08-12Merge bitcoin/bitcoin#30524: doc: rpc: Use "output script" consistently (2/2)merge-script
fa5755b0a8536b844fdccfecf386c1baab24f1c9 doc: rpc: Use "output script" consistently (2/2) (MarcoFalke) Pull request description: Small follow-up to https://github.com/bitcoin/bitcoin/pull/30408 to fixup the RPCs that were forgotten. ACKs for top commit: theStack: lgtm ACK fa5755b0a8536b844fdccfecf386c1baab24f1c9 Tree-SHA512: f1fc0aabb59017da216d6fe0f08a2274336d04db332ad6ce3d9608cd6f03667be1c76423f24a489ac8e7d536011a129dca752ab64b4621b7bc1d4d53f68602e4
2024-08-09Merge bitcoin/bitcoin#30598: assumeutxo: Drop block height from metadataAva Chow
00618e8745192d209c23e3ae873c077e58168957 assumeutxo: Drop block height from metadata (Fabian Jahr) Pull request description: Fixes https://github.com/bitcoin/bitcoin/issues/30514 which has more context and shows how the issue can be reproduced. Since the value in question is removed, there is no test to add to reproduce anything. This is an alternative approach to #30516 with much of the [code being suggested there](https://github.com/bitcoin/bitcoin/pull/30516#discussion_r1689146902). ACKs for top commit: maflcko: re-ACK 00618e8745192d209c23e3ae873c077e58168957 🎌 achow101: ACK 00618e8745192d209c23e3ae873c077e58168957 theStack: Code-review ACK 00618e8745192d209c23e3ae873c077e58168957 ismaelsadeeq: Re-ACK 00618e8745192d209c23e3ae873c077e58168957 mzumsande: ACK 00618e8745192d209c23e3ae873c077e58168957 Tree-SHA512: db9575247bae838ad7742a27a216faaf55bb11e022f9afdd05752bb09bbf9614717d0ad64304ff5722a16bf41d8dea888af544e4ae26dcaa528c1add0269a4a8
2024-08-08assumeutxo: Drop block height from metadataFabian Jahr
The Snapshot format version is updated to 2 to indicate this change. Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
2024-08-07Merge bitcoin/bitcoin#30525: doc, rpc : `#30275` followupsAva Chow
fa2f26960ee084971ab09959b213a9b8104482e5 [rpc, fees]: add more detail on the fee estimation modes (ismaelsadeeq) 6e7e620864cc7a2f3c3b576588afe4d44dc394ec [doc]: add `30275` release notes (ismaelsadeeq) Pull request description: This PR: 1. Adds release notes for #30275 2. Describe fee estimation modes in RPC help texts ACKs for top commit: achow101: ACK fa2f26960ee084971ab09959b213a9b8104482e5 glozow: ACK fa2f26960ee084971ab09959b213a9b8104482e5 willcl-ark: ACK fa2f26960ee tdb3: re ACK fa2f26960ee084971ab09959b213a9b8104482e5 Tree-SHA512: b8ea000b599297b954dc770137c29b47153e68644c58550a73e34b74ecb8b65e78417875481efdfdf6aab0018a9cd1d90d8baa5a015e70aca0975f6e1dc9598c
2024-08-05rpc: clarify ALREADY_IN_CHAIN rpc errorswillcl-ark
When using `sendrawtransaction` the ALREADY_IN_CHAIN error help string may be confusing. Rename TransactionError::ALREADY_IN_CHAIN to TransactionError::ALREADY_IN_UTXO_SET and update the rpc help string. Remove backwards compatibility alias as no longer required.
2024-08-05Merge bitcoin/bitcoin#29656: chainparams: Change nChainTx type to uint64_tglozow
bf0efb4fc72d3c49a2c498c944e55466dfa046dc scripted-diff: Modernize naming of nChainTx and nTxCount (Fabian Jahr) 72e5d1be1f4491565249d43e836ee42cfd858866 test: Add basic check for nChainTx type (Fabian Jahr) dc2938e9799d79696d1db2438ef33d90542d984b chainparams: Change nChainTx to uint64_t (Fabian Jahr) Pull request description: This picks up the work from #29331 and closes #29258. This simply changes the type and addresses the comments from #29331 by changing the type in all relevant places and removing unnecessary casts. This also adds an extremely simple unit test. Additionally this modernizes the name of `nChainTx` which helps reviewers check all use of the symbol and can make silent merge conflicts. ACKs for top commit: maflcko: only rebase in scripted-diff, re-ACK bf0efb4fc72d3c49a2c498c944e55466dfa046dc 🔈 glozow: reACK bf0efb4fc72 via range-diff Tree-SHA512: ee4020926d0800236fe655d0c7b127215ab36b553b04d5f91494f4b7fac6e1cfe7ee298b07c0983db5a3f4786932acaa54f5fd2ccd45f2fcdcfa13427358dc3b
2024-08-04Merge bitcoin/bitcoin#29798: Logging cleanupRyan Ofsky
a7432dd6ed3e13a272d62ecde535e6d562cc932c logging: clarify -debug and -debugexclude descriptions (Anthony Towns) 74dd33cb0a967086df32e5140d58843ca1359d81 rpc: make logging method reject "0" category and correct the help text (Vasil Dimov) 8c6f3bf1634533a0dd268dcf5929e49429640a09 logging, refactor: minor encapsulation improvement and use BCLog::NONE instead of 0 (Vasil Dimov) 160706aa387245ed96b1f13e5362fe1837e8fc4b logging, refactor: make category special cases explicit (Ryan Ofsky) Pull request description: * Move special cases from `LOG_CATEGORIES_BY_STR` to `GetLogCategory()` (suggested [here](https://github.com/bitcoin/bitcoin/pull/29419#discussion_r1547990373)). * Remove `"none"` and `"0"` from RPC `logging` help because that help text was wrong. `"none"` resulted in an error and `"0"` was ignored itself (contrary to what the help text suggested). * Remove unused `LOG_CATEGORIES_BY_STR[""]` (suggested [here](https://github.com/bitcoin/bitcoin/pull/29419#discussion_r1548018694)). This is a followup to https://github.com/bitcoin/bitcoin/pull/29419, addressing leftover suggestions + more. ACKs for top commit: LarryRuane: ACK a7432dd6ed3e13a272d62ecde535e6d562cc932c ryanofsky: Code review ACK a7432dd6ed3e13a272d62ecde535e6d562cc932c. Only changes since last review are removing dead if statement and adding AJ's suggested -debug and -debugexclude help improvements, which look accurate and much more clear. Tree-SHA512: 41b997b06fccdb4c1d31f57d4752c83caa744cb3280276a337ef4a9b7012a04eb945071db6b8fad24c6a6cf8761f2f800fe6d8f3d8836f5b39c25e4f11c85bf0
2024-08-04scripted-diff: Modernize naming of nChainTx and nTxCountFabian Jahr
-BEGIN VERIFY SCRIPT- sed -i 's/nChainTx/m_chain_tx_count/g' $(git grep -l 'nChainTx' ./src) sed -i 's/nTxCount/tx_count/g' $(git grep -l 'nTxCount' ./src) -END VERIFY SCRIPT-
2024-08-04rpc: make logging method reject "0" category and correct the help textVasil Dimov
Current logging RPC method documentation claims to accept "0" and "none" categories, but the "none" argument is actually rejected and the "0" argument is ignored. Update the implementation to refuse both categories, and remove the help text claiming to support them.
2024-08-02Merge bitcoin/bitcoin#30352: policy: Add PayToAnchor(P2A), `OP_1 <0x4e73>` ↵glozow
as a standard output script for spending 75648cea5a9032b3d388cbebacb94d908e08924e test: add P2A ProduceSignature coverage (Greg Sanders) 7998ce6b20fba62c022228355907b612ba6692e1 Add release note for P2A output feature (Greg Sanders) 71c9b02a04742eeecab14aae4697b1a3eb51ff7f test: add P2A coverage for decodescript (Greg Sanders) 1349e9ec1558484f2912a2444c410170fcec8745 test: Add anchor mempool acceptance test (Greg Sanders) 9d892099378b2ad5f52220403bdeae43c61d6955 policy: stop 3rd party wtxid malleability of anchor spend (Greg Sanders) b60aaf8b239978947d2b0e3f56e7d8a4092d7570 policy: make anchor spend standard (Greg Sanders) 455fca86cfada1823aa28615b5683f9dc73dbb9a policy: Add OP_1 <0x4e73> as a standard output type (Greg Sanders) Pull request description: This is a sub-feature taken out of the original proposal for ephemeral anchors #30239 This PR makes *spending* of `OP_1 <0x4e73>` (i.e. `bc1pfeessrawgf`) standard. Creation of this output type is already standard. Any future witness output types are considered relay-standard to create, but not to spend. This preserves upgrade hooks, such as a completely new output type for a softfork such as BIP341. It also gives us a bit of room to use a new output type for policy uses. This particular sized witness program has no other known use-cases (https://bitcoin.stackexchange.com/a/110664/17078), s it affords insufficient cryptographic security for a secure commitment to data, such as a script or a public key. This makes this type of output "keyless", or unauthenticated. As a witness program, the `scriptSig` of the input MUST be blank, by BIP141. This helps ensure txid-stability of the spending transaction, which may be required for smart contracting wallets. If we do not use segwit, a miner can simply insert an `OP_NOP` in the `scriptSig` without effecting the result of program execution. An additional relay restriction is to disallow non-empty witness data, which an adversary may use to penalize the "honest" transactor when RBF'ing the transaction due to the incremental fee requirement of RBF rules. The intended use-case for this output type is to "anchor" the transaction with a spending child to bring exogenous CPFP fees into the transaction package, encouraging the inclusion of the package in a block. The minimal size of creation and spending of this output makes it an attractive contrast to outputs like `p2sh(OP_TRUE)` and `p2wsh(OP_TRUE)` which are significantly larger in vbyte terms. Combined with TRUC transactions which limits the size of child transactions significantly, this is an attractive option for presigned transactions that need to be fee-bumped after the fact. ACKs for top commit: sdaftuar: utACK 75648cea5a9032b3d388cbebacb94d908e08924e theStack: re-ACK 75648cea5a9032b3d388cbebacb94d908e08924e ismaelsadeeq: re-ACK 75648cea5a9032b3d388cbebacb94d908e08924e via [diff](https://github.com/bitcoin/bitcoin/compare/e7ce6dc070c0319cbb868d41cadd836b2e6ca9db..75648cea5a9032b3d388cbebacb94d908e08924e) glozow: ACK 75648cea5a9032b3d388cbebacb94d908e08924e tdb3: ACK 75648cea5a9032b3d388cbebacb94d908e08924e Tree-SHA512: d529de23d20857e6cdb40fa611d0446b49989eaafed06c28280e8fd1897f1ed8d89a4eabbec1bbf8df3d319910066c3dbbba5a70a87ff0b2967d5205db32ad1e
2024-08-02[rpc, fees]: add more detail on the fee estimation modesismaelsadeeq
- Add description that indicates the fee estimation modes behaviour. - This description will be returned in the RPC's help texts.
2024-08-02Merge bitcoin/bitcoin#30544: rpc: fix maybe-uninitialized compile warning in ↵merge-script
getchaintxstats 2e86f2b2019ea0edbd89dd3fd13540c5bbfa104d rpc: fix maybe-uninitialized compile warning in getchaintxstats (Michael Dietz) Pull request description: This resolves the compiler warning about potential uninitialized use of window_tx_count introduced in fa2dada. The warning: ``` CXX rpc/libbitcoin_node_a-blockchain.o rpc/blockchain.cpp: In function ‘getchaintxstats()::<lambda(const RPCHelpMan&, const JSONRPCRequest&)>’: rpc/blockchain.cpp:1742:38: warning: ‘*(std::_Optional_payload_base<unsigned int>::_Storage<unsigned int, true>*)((char*)&window_tx_count + offsetof(const std::optional<unsigned int>,std::optional<unsigned int>::<unnamed>.std::_Optional_base<unsigned int, true, true>::<unnamed>)).std::_Optional_payload_base<unsigned int>::_Storage<unsigned int, true>::_M_value’ may be used uninitialized in this function [-Wmaybe-uninitialized] 1742 | ret.pushKV("txrate", double(*window_tx_count) / nTimeDiff); | ``` ACKs for top commit: maflcko: lgtm ACK 2e86f2b2019ea0edbd89dd3fd13540c5bbfa104d theStack: ACK 2e86f2b2019ea0edbd89dd3fd13540c5bbfa104d tdb3: ACK 2e86f2b2019ea0edbd89dd3fd13540c5bbfa104d Tree-SHA512: c087e8f1cd68dd8df734a8400d30a95abe57ebd56cd53aef4230e425b33a23aa55b3af42abfd162e3be8c937a4c27e56abb70a4fedb10e2df64d52d577e0f262
2024-07-30policy: Add OP_1 <0x4e73> as a standard output typeGreg Sanders
These outputs are called anchors, and allow key-less anchor spends which are vsize-minimized versus keyed anchors which require larger outputs when creating and inputs when spending.
2024-07-29rpc: fix maybe-uninitialized compile warning in getchaintxstatsMichael Dietz
This resolves the compiler warning about potential uninitialized use of window_tx_count introduced in fa2dada.
2024-07-28Merge bitcoin/bitcoin#30515: rpc: add utxo's blockhash and number of ↵merge-script
confirmations to scantxoutset output 17845e7f219e2281cd7a51d2cfe67b22eb40c4ba rpc: add utxo's blockhash and number of confirmations to scantxoutset output (Luis Schwab) Pull request description: This PR resolves #30478 by adding two fields to the `scantxoutset` RPC: - blockhash: the blockhash that an UTXO was created - confirmations: the number of confirmations an UTXO has relative to the chaintip. The rationale for the first field is that a blockhash is a much more reliable identifier than the height: > When using the scantxoutset RPC, the current behaviour is to show the block height of the UTXO. This is not optimal, as block height is ambiguous, especially in the case of a block reorganization happening at the same instant of the query. In this case, an UTXO that does not exist would be assumed to exist, unless the chain's tip hash is recorded before the scan, and make sure it still exists after, as per https://github.com/bitcoindevkit/bdk/issues/895#issuecomment-1475766797 comment by evanlinjin. The second one was suggested by maflcko, and I agree it's useful for human users: > While touching this, another thing to add could be the number of confirmations? I understand that this wouldn't help machine consumers of the interface, but human callers may find it useful? This will yield an RPC output like so: ```diff bitcoin-cli scantxoutset start "[\"addr(bc1q5q9344vdyjkcgv79ve3tldz4jmx4lf7knmnx6r)\"]" { "success": true, "txouts": 185259116, "height": 853622, "bestblock": "00000000000000000002e97d9be8f0ddf31829cf873061b938c10b0f80f708b2", "unspents": [ { "txid": "fae435084345fe26e464994aebc6544875bca0b897bf4ce52a65901ae28ace92", "vout": 0, "scriptPubKey": "0014a00b1ad58d24ad8433c56662bfb45596cd5fa7d6", "desc": "addr(bc1q5q9344vdyjkcgv79ve3tldz4jmx4lf7knmnx6r)#smk4xmt7", "amount": 0.00091190, "coinbase": false, "height": 852741, + "blockhash": "00000000000000000002eefe7e7db44d5619c3dace4c65f3fdcd2913d4945c13", + "confirmations": 882 } ], "total_amount": 0.00091190 } ``` ACKs for top commit: sipa: utACK 17845e7f219e2281cd7a51d2cfe67b22eb40c4ba Eunovo: ACK https://github.com/bitcoin/bitcoin/pull/30515/commits/17845e7f219e2281cd7a51d2cfe67b22eb40c4ba tdb3: ACK 17845e7f219e2281cd7a51d2cfe67b22eb40c4ba Tree-SHA512: 02366d0004e5d547522115ef0efe6794a35978db53dda12c675cfae38197bf43f0bf89ca99a3d79e3d2cff95186015fe1ab764abb8ab82bda440ae9302ad973b
2024-07-27rpc: add utxo's blockhash and number of confirmations to scantxoutset outputLuis Schwab
2024-07-25doc: rpc: Use "output script" consistently (2/2)MarcoFalke
2024-07-25Merge bitcoin/bitcoin#30482: rest: Reject truncated hex txid early in ↵merge-script
getutxos parsing fac0c3d4bfc97b94f0594f7606650921feef2c8a doc: Add release notes for two pull requests (MarcoFalke) fa7b57e5f5a6dafbbadc361ffd27b58afff1ed59 refactor: Replace ParseHashStr with FromHex (MarcoFalke) fa9077724507faad207f29509a8202fc6ac9d502 rest: Reject truncated hex txid early in getutxos parsing (MarcoFalke) fab6ddbee64e50d5e2f499aebca35b5911896ec4 refactor: Expose FromHex in transaction_identifier (MarcoFalke) fad2991ba073de0bd1f12e42bf0fbaca4a265508 refactor: Implement strict uint256::FromHex() (MarcoFalke) fa103db2bb736bce4440f0bde564e6671e36311d scripted-diff: Rename SetHex to SetHexDeprecated (MarcoFalke) fafe4b80512a5a82712a3ee81b68cfeb21271dee test: refactor: Replace SetHex with uint256 constructor directly (MarcoFalke) Pull request description: In `rest_getutxos` truncated txids such as `aa` or `ff` are accepted. This is brittle at best. Fix it by rejecting any truncated (or overlarge) input. ---- Review note: This also starts a major refactor to rework hex parsing in Bitcoin Core, meaning that a few refactor commits are included as well. They are explained individually in the commit message and the work will be continued in the future. ACKs for top commit: stickies-v: re-ACK fac0c3d4bfc97b94f0594f7606650921feef2c8a - only doc and test updates to address review comments, thanks! hodlinator: ACK fac0c3d4bfc97b94f0594f7606650921feef2c8a Tree-SHA512: 473feb3fcf6118443435d1dd321006135b0b54689bfbbcb1697bb5811a449bef51f475c715de6911ff3c4ea3bdb75f601861ff93347bc4414d6b9e5298105dd7
2024-07-25Merge bitcoin/bitcoin#30275: Fee Estimation: change `estimatesmartfee` ↵merge-script
default mode to `economical` 25bf86a225b0df3f48ade1016b47f5ee1636b988 [test]: ensure `estimatesmartfee` default mode is `economical` (ismaelsadeeq) 41a2545046bce315af697a3c6baf6e3fb2e824c2 [fees]: change `estimatesmartfee` default mode to `economical` (ismaelsadeeq) Pull request description: Fixes #30009 This PR changes the `estimatesmartfee` default mode to `economical`. This was also suggested on IRC https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2024-04-26#1021609 - `conservative` mode: This is the `estimatesmartfee` RPC mode which considers a longer history of blocks. It potentially returns a higher fee rate and is more likely to be sufficient for the desired target, but it is not as responsive to short-term drops in the prevailing fee market. - `economical` mode: This is the `estimatesmartfee` RPC mode where estimates are potentially lower and more responsive to short-term drops in the prevailing fee market. Since users are likely to use the default mode, this change will reduce overestimation for many users. The conservative mode remains available for those who wish to opt-in. For an in-depth analysis of how significantly the `conservative` mode overestimates, see https://delvingbitcoin.org/t/bitcoind-policy-estimator-modes-analysis/964. ACKs for top commit: instagibbs: reACK https://github.com/bitcoin/bitcoin/pull/30275/commits/25bf86a225b0df3f48ade1016b47f5ee1636b988 glozow: ACK 25bf86a225b0df3f48ade1016b47f5ee1636b988 willcl-ark: ACK 25bf86a225b0df3f48ade1016b47f5ee1636b988 Tree-SHA512: 78ebda667eb9c8f87dcc2f0e6c14968bd1de30358dc77a13611b186fb8427ad97d9f537bad6e32e0a1aa477ccd8c64fee4d41e19308ef3cb184ff1664e6ba8a6
2024-07-24refactor: Replace ParseHashStr with FromHexMarcoFalke
No need to have two functions with different names that achieve the exact same thing.
2024-07-23Merge bitcoin/bitcoin#30408: rpc: doc: use "output script" terminology ↵Ava Chow
consistently in "asm"/"hex" results 29eafd5733d77b3e8f3f3ab6cd65c61ac0e8536b rpc: doc: use "output script" terminology consistently in "asm"/"hex" results (Sebastian Falbesoner) Pull request description: The wording "public key script" was likely chosen as a human-readable form of the technical term `scriptPubKey`, but it doesn't seem to be really widespread. Replace it by the more (probably most?) common term "output script" instead. Note that the argument for the `decodescript` RPC is not necessarily an output script (it could e.g. be also a redeem script), so in this case we just stay generic and use "script". See also the draft BIP "Terminology for Transaction Components" (https://github.com/murchandamus/bips/blob/2022-04-tx-terminology/bip-tx-terminology.mediawiki) from murchandamus which suggests to use "output script" as well. Affects the help text of the following RPCs: - decodepsbt - decoderawtransaction - decodescript - getblock (if verbosity=3) - getrawtransaction (if verbosity=2,3) - gettxout ACKs for top commit: maflcko: ACK 29eafd5733d77b3e8f3f3ab6cd65c61ac0e8536b achow101: ACK 29eafd5733d77b3e8f3f3ab6cd65c61ac0e8536b BrandonOdiwuor: ACK 29eafd5733d77b3e8f3f3ab6cd65c61ac0e8536b tdb3: ACK 29eafd5733d77b3e8f3f3ab6cd65c61ac0e8536b Tree-SHA512: 62eb92d42bc44e36dc3090df7b248a123868a74af253d2046de02086e688bf6ff98307b927ba2fee3d599f85e073aeb8eca90ed15105ca63b648b6796cfa340b
2024-07-18Merge bitcoin/bitcoin#30356: refactor: add coinbase constraints to ↵Ryan Ofsky
BlockAssembler::Options c504b6997b1acc9771ad1f52efaa4be2b4966c6c refactor: add coinbase constraints to BlockCreateOptions (Sjors Provoost) 6b4c817d4b978adf69738677c74855ef0675f333 refactor: pass BlockCreateOptions to createNewBlock (Sjors Provoost) 323cfed5959b25c98235ec988b408fc5e3391e3c refactor: use CHECK_NONFATAL to avoid single-use symbol (Sjors Provoost) Pull request description: When generating a block template through e.g. getblocktemplate RPC, we reserve 4000 weight units and 400 sigops. Pools use this space for their coinbase outputs. At least one pool patched their Bitcoin Core node to adjust these hardcoded values. They eventually [produced an invalid block](https://bitcoin.stackexchange.com/questions/117837/how-many-sigops-are-in-the-invalid-block-783426) which exceeded the sigops limit. The existince of such patches suggests it may be useful to make this value configurable. This PR would make such a change easier. However, the main motivation is that in the Stratum v2 spec requires the pool to communicate the maximum bytes they intend to add to the coinbase outputs. Specifically the `CoinbaseOutputDataSize` message which is part of the [Template Distribution Protocol](https://github.com/stratum-mining/sv2-spec/blob/main/07-Template-Distribution-Protocol.md#71-coinbaseoutputdatasize-client---server) has a field `coinbase_output_max_additional_size`. A proposed change to the spec adds the max additional sigops as well: https://github.com/stratum-mining/sv2-spec/pull/86. Whether that change makes it into the spec is not important though, as adding both to `BlockAssembler::Options` makes sense. The first commit is a test refactor followup for #30335, related to the code that's changed here, but not required. The second commit introduces BlockCreateOptions, with just `use_mempool`. The thirds commit adds `coinbase_max_additional_weight` and `coinbase_output_max_additional_sigops` to `BlockCreateOptions`. They use the originally hardcoded values, and no existing caller overrides these defaults. This changes in #29432. ACKs for top commit: itornaza: tested ACK c504b6997b1acc9771ad1f52efaa4be2b4966c6c ryanofsky: Code review ACK c504b6997b1acc9771ad1f52efaa4be2b4966c6c ismaelsadeeq: Code review ACK c504b6997b1acc9771ad1f52efaa4be2b4966c6c Tree-SHA512: de2fa085f47048c91d95524e03f909f6f27f175c1fefa3d6106445e7eb5cf5b710eda6ea5b641cf3b4704a4e4e0181a0c829003b9fd35465f2a46167e5d64487
2024-07-18[fees]: change `estimatesmartfee` default mode to `economical`ismaelsadeeq
2024-07-17Merge bitcoin/bitcoin#29523: Wallet: Add `max_tx_weight` to transaction ↵Ava Chow
funding options (take 2) 734076c6de1781f957c8bc3bf7ed6951920cfcf6 [wallet, rpc]: add `max_tx_weight` to tx funding options (ismaelsadeeq) b6fc5043c16c2467a2a6768a6ca9b18035fc400f [wallet]: update the data type of `change_output_size`, `change_spend_size` and `tx_noinputs_size` to `int` (ismaelsadeeq) baab0d2d43049a71dc90176bc4d72062f7b2ce19 [doc]: update reason for deducting change output weight (ismaelsadeeq) 7f61d31a5cec8fc61328bee43f90d3f1dcb0a035 [refactor]: update coin selection algorithms input parameter `max_weight` name (ismaelsadeeq) Pull request description: This PR taken over from #29264 The PR added an option `max_tx_weight` to transaction funding RPC's that ensures the resulting transaction weight does not exceed the specified `max_tx_weight` limit. If `max_tx_weight` is not given `MAX_STANDARD_TX_WEIGHT` is used as the max threshold. This PR addressed outstanding review comments in #29264 For more context and rationale behind this PR see https://delvingbitcoin.org/t/lightning-transactions-with-v3-and-ephemeral-anchors/418/11?u=instagibbs ACKs for top commit: achow101: ACK 734076c6de1781f957c8bc3bf7ed6951920cfcf6 furszy: utACK 734076c6de1781f957c8bc3bf7ed6951920cfcf6 rkrux: reACK [734076c](https://github.com/bitcoin/bitcoin/pull/29523/commits/734076c6de1781f957c8bc3bf7ed6951920cfcf6) Tree-SHA512: 013501aa443d239ee2ac01bccfc5296490c27b4edebe5cfca6b96c842375e895e5cfeb5424e82e359be581460f8be92095855763a62779a18ccd5bdfdd7ddce7
2024-07-16Merge bitcoin/bitcoin#30429: rpc: Use CHECK_NONFATAL over AssertAva Chow
fa6270737eb9655bfb4e29b7070ecb6cd2087b7f rpc: Use CHECK_NONFATAL over Assert (MarcoFalke) Pull request description: Any RPC method should not abort the whole node when an internal logic error happens. Fix it by just aborting this single RPC method call when an error happens. Also, fix the linter to find the fixed cases. ACKs for top commit: achow101: ACK fa6270737eb9655bfb4e29b7070ecb6cd2087b7f stickies-v: ACK fa6270737eb9655bfb4e29b7070ecb6cd2087b7f tdb3: ACK fa6270737eb9655bfb4e29b7070ecb6cd2087b7f hodlinator: ACK fa6270737eb9655bfb4e29b7070ecb6cd2087b7f Tree-SHA512: dad2f31b01a66578949009499e4385fb4d72f0f897419f2a6e0ea02e799b9a31e6ecb5a67fa5d27fcbc7939fe8acd62dc04e877b35831493b7f2c604dec7dc64
2024-07-16refactor: pass BlockCreateOptions to createNewBlockSjors Provoost
Rather than pass options individually to createNewBlock and then combining them into BlockAssembler::Options, this commit introduces BlockCreateOptions and passes that instead. Currently there's only one option (use_mempool) but the next commit adds more. Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2024-07-16refactor: use CHECK_NONFATAL to avoid single-use symbolSjors Provoost
2024-07-12rpc: Use CHECK_NONFATAL over AssertMarcoFalke
2024-07-11Merge bitcoin/bitcoin#30406: refactor: modernize-use-equals-defaultmerge-script
3333bae9b2a6c1ee2314d33361c93944c12001f9 tidy: modernize-use-equals-default (MarcoFalke) Pull request description: Prior to C++20, `modernize-use-equals-default` could have been problematic because it could turn a non-aggregate into an aggregate. The risk would be that aggregate initialization would be enabled where the author did not intend to enable it. With C++20, aggregate for those is forbidden either way. (https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p1008r1.pdf) So enabled it for code clarity, consistency, and possibly unlocking compiler optimizations. See https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-equals-default.html ACKs for top commit: stickies-v: ACK 3333bae9b2a6c1ee2314d33361c93944c12001f9 Tree-SHA512: ab42ff01be7ca7e7d8b4c6a485e68426f59627d83dd827cf292304829562348dc17a52ee009f5f6f3c1c2081d7166ffac4baef23197ebeba8de7767c6ddfe255
2024-07-10Merge bitcoin/bitcoin#29668: prune, rpc: Check undo data when finding ↵Ava Chow
pruneheight 8789dc8f315a9d9ad7142d831bc9412f780248e7 doc: Add note to getblockfrompeer on missing undo data (Fabian Jahr) 4a1975008b602aeacdad9a74d1837a7455148074 rpc: Make pruneheight also reflect undo data presence (Fabian Jahr) 96b4facc912927305b06a233cb8b36e7e5964c08 refactor, blockstorage: Generalize GetFirstStoredBlock (Fabian Jahr) Pull request description: The function `GetFirstStoredBlock()` helps us find the first block for which we have data. So far this function only looked for a block with `BLOCK_HAVE_DATA`. However, this doesn't mean that we also have the undo data of that block, and undo data might be required for what a user would like to do with those blocks. One example of how this might happen is if some blocks were fetched using the `getblockfrompeer` RPC. Blocks fetched from a peer will have data but no undo data. The first commit here allows `GetFirstStoredBlock()` to check for undo data as well by passing a parameter. This alone is useful for #29553 and I would use it there. In the second commit I am applying the undo check to the RPCs that report `pruneheight` to the user. I find this much more intuitive because I think the user expects to be able to do all operations on blocks up until the `pruneheight` but that is not the case if undo data is missing. I personally ran into this once before and now again when testing for assumeutxo when I had used `getblockfrompeer`. The following commit adds test coverage for this change of behavior. The last commit adds a note in the docs of `getblockfrompeer` that undo data will not be available. ACKs for top commit: achow101: ACK 8789dc8f315a9d9ad7142d831bc9412f780248e7 furszy: Code review ACK 8789dc8f315a9d9ad7142d831bc9412f780248e7. stickies-v: ACK 8789dc8f315a9d9ad7142d831bc9412f780248e7 Tree-SHA512: 90ae8bdd07a496ade579aa25240609c61c9ed173ad38d30533f6c631fe674e5a41727478ade69ca4b71a571ad94c9da4b33ebba6b5d8821109313c2de3bdfb3d
2024-07-09Merge bitcoin/bitcoin#30396: random: add benchmarks and drop unnecessary ↵Ava Chow
Shuffle function 6ecda04fefad980872c72fba89844393f5581120 random: drop ad-hoc Shuffle in favor of std::shuffle (Pieter Wuille) da28a26aae3178fb7663efbe20bb650857ace775 bench random: benchmark more functions, and add InsecureRandomContext (Pieter Wuille) 0a9bbc64c157a314e5472ecd98300e30b12d3fdf random bench refactor: move to new bench/random.cpp (Pieter Wuille) Pull request description: This adds benchmarks for various operations on `FastRandomContext` and `InsecureRandomContext`, and then removes the ad-hoc `Shuffle` functions, now that it appears that standard library `std::shuffle` has comparable performance. The other reason for keeping `Shuffle`, namely the fact that libstdc++ used self-move (which debug mode panics on) has been fixed as well (see https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1658344049). ACKs for top commit: achow101: ACK 6ecda04fefad980872c72fba89844393f5581120 hodlinator: ACK 6ecda04fefad980872c72fba89844393f5581120 dergoegge: Code review ACK 6ecda04fefad980872c72fba89844393f5581120 Tree-SHA512: 2560b7312410581ff2b9bd0716e0f1558d910b5eadb9544785c972384985ac0f11f72d6b2797cfe2e7eb71fa57c30cffd98cc009cb4ee87a18b1524694211417
2024-07-08rpc: doc: use "output script" terminology consistently in "asm"/"hex" resultsSebastian Falbesoner
The wording "public key script" was likely chosen as a human-readable form of the technical term `scriptPubKey`, but it doesn't seem to be really widespread. Replace it by the more common term "output script" instead. Note that the argument for the `decodescript` RPC is not necessarily an output script (it could e.g. be also a redeem script), so in this case we just stay generic and use "script". See also the draft BIP "Terminology for Transaction Components" (https://github.com/murchandamus/bips/blob/2022-04-tx-terminology/bip-tx-terminology.mediawiki) which suggests to use "output script" as well. Affects the help text of the following RPCs: - decodepsbt - decoderawtransaction - decodescript - getblock (if verbosity=3) - getrawtransaction (if verbosity=2,3) - gettxout
2024-07-08tidy: modernize-use-equals-defaultMarcoFalke
2024-07-06random: drop ad-hoc Shuffle in favor of std::shufflePieter Wuille
Benchmarks show it is no longer faster with modern standard C++ libraries, and the debug-mode failure due to self-move has been fixed as well.
2024-07-05rpc: Use untranslated error strings in loadtxoutsetMarcoFalke
2024-07-05refactor: Use named arguments to get path arg in loadtxoutsetMarcoFalke
2024-07-02Merge bitcoin/bitcoin#29720: rpc: Avoid getchaintxstats invalid resultsAva Chow
2342b46c451658a418f8e28e50b2ad0e5abd284d test: Add coverage for getchaintxstats in assumeutxo context (Fabian Jahr) faf2a6750b2da97a18c48a3acf9c9da2aebe86d0 rpc: Reorder getchaintxstats output (MarcoFalke) fa2dada0c9ab61266bcca86fcd28ced873976916 rpc: Avoid getchaintxstats invalid results (MarcoFalke) Pull request description: The `getchaintxstats` RPC reply during AU background download may return non-zero, but invalid, values for `window_tx_count` and `txrate`. For example, `txcount` may be zero for a to-be-downloaded block, but may be non-zero for an ancestor block which is already downloaded. Thus, the values returned may be negative (and cause intermediate integer sanitizer violations). Also, `txcount` may be accurate for the snapshot base block, or a descendant of it. However it may be zero for an ancestor block that still needs to be downloaded. Thus, the values returned may be positive, but wrong. Fix all issues by skipping the returned value if either `txcount` is unset (equal to zero). Also, skip `txcount` in the returned value, if it is unset (equal to zero). Fixes https://github.com/bitcoin/bitcoin/issues/29328 ACKs for top commit: fjahr: re-ACK 2342b46c451658a418f8e28e50b2ad0e5abd284d achow101: ACK 2342b46c451658a418f8e28e50b2ad0e5abd284d mzumsande: ACK 2342b46c451658a418f8e28e50b2ad0e5abd284d Tree-SHA512: 931cecc40ee5dc0f96be728db7eb297155f8343076cd29c8b8c050c99fd1d568b80f54c9459a34ca7a9489c2474c729796d00eeb1934d6a9f7b4d6a53e3ec430
2024-07-02Merge bitcoin/bitcoin#30267: assumeutxo: Check snapshot base block is not in ↵Ava Chow
invalid chain 2f9bde69f45c7a9fdcf0c65f9e1305391a6f1f28 test: Remove unnecessary restart in assumeutxo test (Fabian Jahr) 19ce3d407ef546fa50d18b2ffbd67b7417797064 assumeutxo: Check snapshot base block is not marked invalid (Fabian Jahr) 80315c011863d69e7785673283e4c9033fbcd5ac refactor: Move early loadtxoutset checks into ActiveSnapshot (Fabian Jahr) Pull request description: This was discovered in a discussion in #29996 If the base block of the snapshot is marked invalid or part of an invalid chain, we currently still load the snapshot and get stuck in a weird state where we have the snapshot chainstate but it will never connect to our valid chain. While this scenario is highly unlikely to occur on mainnet, it still seems good to prevent this inconsistent state. The behavior change described above is in the second commit. The first commit refactors the early checks in the `loadtxoutset` RPC by moving them into `ActivateSnapshot()` in order to have the chance to cover them by unit tests in the future and have a more consistent interface. Previously checks were spread out between `rpc/blockchain.cpp` and `validation.cpp`. In order to be able to return the error message to users of the RPC, the return type of `ActivateSnapshot()` is changed from `bool` to `util::Result`. The third commit removes an unnecessary restart introduced in #29428. ACKs for top commit: mzumsande: re-ACK 2f9bde6 alfonsoromanz: Re-ACK 2f9bde69f45c7a9fdcf0c65f9e1305391a6f1f28. The RPC code looks much cleaner after the refactor. Also, it seems very useful to get the error message in the RPC response rather than having to rely on the logs in some scenarios if you are an RPC user. achow101: ACK 2f9bde69f45c7a9fdcf0c65f9e1305391a6f1f28 Tree-SHA512: 5328dd88c3c7be3f1be97c9eef52ac3666c27188c30a798b3e949f3ffcb83be075127c107e4046f7f39f961a79911ea3d61b61f3c11e451b3e4c541c264eeed4
2024-07-02Merge bitcoin/bitcoin#30344: kernel: remove mempool_persistglozow
f1478c05458562a9bef5c2ba43959d758e7b4745 mempool: move LoadMempool/DumpMempool to node (Cory Fields) 6d242ff1e9ca02fd8f5cb3ffe82dfb48a52366cc kernel: remove mempool_persist.cpp (Cory Fields) Pull request description: DumpMempool/LoadMempool are not necessary for the kernel. Noticed while working on instantiated logging. I suppose these could have been left in on purpose, but I'm assuming it was probably just an oversight. ACKs for top commit: TheCharlatan: Re-ACK f1478c05458562a9bef5c2ba43959d758e7b4745 glozow: ACK f1478c0545 stickies-v: ACK f1478c05458562a9bef5c2ba43959d758e7b4745 Tree-SHA512: 5825da0cf2e67470524eb6ebe397eb90755a368469a25f184df99ab935b3eb6d89eb802b41a6c3661e869bba3bbfa8ba9d95281bc75ebbf790ec5d9d1f79c66f
2024-07-02rpc: Reorder getchaintxstats outputMarcoFalke
2024-07-02rpc: Avoid getchaintxstats invalid resultsMarcoFalke
2024-06-27Merge bitcoin/bitcoin#30335: Mining interface followups, reduce cs_main ↵Ryan Ofsky
locking, test rpc bug fix a74b0f93efa1d9eaf5abc2f6591c44a632aec6ed Have testBlockValidity hold cs_main instead of caller (Sjors Provoost) f6dc6db44ddc22ade96a69a02908f14cfb279a37 refactor: use CHECK_NONFATAL to avoid single-use symbol (Sjors Provoost) 5fb2b704897fe10b5bd5ed754a5afd2ddc4a9e1d Drop unneeded lock from createNewBlock (Sjors Provoost) 75ce7637ad75af890581660c0bb3565c3c03bd6c refactor: testBlockValidity make out argument last (Sjors Provoost) 83a9bef0e2acad7655e23d30e1c52412f380d93d Add missing include for mining interface (Sjors Provoost) Pull request description: Followups from #30200 Fixes: - `std::unique_ptr` needs `#include <memory>` (noticed while working on #30332, which has fewer includes than its parent PR that I originally tested with) - Drop lock from createNewBlock that was spuriously added - Have testBlockValidity hold cs_main instead of caller (also fixes a race condition in test-only code) Refactor: - Use CHECK_NONFATAL to avoid single-use symbol (refactor) - move output argument `state` to the end of `testBlockValidity`, see https://github.com/bitcoin/bitcoin/pull/30200#discussion_r1647987176 ACKs for top commit: AngusP: Code Review ACK a74b0f93efa1d9eaf5abc2f6591c44a632aec6ed itornaza: Tested ACK a74b0f93efa1d9eaf5abc2f6591c44a632aec6ed ryanofsky: Code review ACK a74b0f93efa1d9eaf5abc2f6591c44a632aec6ed. Just new error string is added since last review, and a commit message was updated Tree-SHA512: 805e133bb59303fcee107d6f02b3e2761396c290efb731a85e6a29ae56b4b1b9cd28ada9629e979704dcfd98cf35034e7e6b618e29923049eb1eca2f65630e41
2024-06-27Merge bitcoin/bitcoin#28167: init: Add option for rpccookie permissions ↵Ryan Ofsky
(replace 26088) 73f0a6cbd0b628675028fbd5a37eff8115e7ccfe doc: detail -rpccookieperms option (willcl-ark) d2afa2690cceb0012b2aa1960e1cfa497f3103fa test: add rpccookieperms test (willcl-ark) f467aede78533dac60a118e1566138d65522c213 init: add option for rpccookie permissions (willcl-ark) 7df03f1a923e239cea8c9b0d603a9eb00863a40c util: add perm string helper functions (willcl-ark) Pull request description: This PR picks up #26088 by aureleoules which adds a bitcoind launch option `-rpccookieperms` to set the file permissions of the cookie generated by bitcoin core. Example usage to make the generated cookie group-readable: `./src/bitcoind -rpccookieperms=group`. Accepted values for `-rpccookieperms` are `[owner|group|all]`. We let `fs::perms` handle platform-specific permissions changes. ACKs for top commit: achow101: ACK 73f0a6cbd0b628675028fbd5a37eff8115e7ccfe ryanofsky: Code review ACK 73f0a6cbd0b628675028fbd5a37eff8115e7ccfe. Main change since last review is no longer throwing a skip exception in the rpc test on windows, so other checks can run after it, and overall test result is passing, not skipped. Also were clarifying renames and documentation improvements. tdb3: cr ACK 73f0a6cbd0b628675028fbd5a37eff8115e7ccfe Tree-SHA512: e800d59a44aca10e1c58ca69bf3fdde9f6ccf5eab4b7b962645af6d6bc0cfa3a357701e409c8c60d8d7744fcd33a91e77ada11790aa88cd7811ef60fab86ab11
2024-06-27[wallet, rpc]: add `max_tx_weight` to tx funding optionsismaelsadeeq
This allows a transaction's weight to be bound under a certain weight if possible and desired. This can be beneficial for future RBF attempts, or whenever a more restricted spend topology is desired. Co-authored-by: Greg Sanders <gsanders87@gmail.com>
2024-06-27init: add option for rpccookie permissionswillcl-ark
Add a bitcoind launch option `-rpccookieperms` to configure the file permissions of the cookie on Unix systems.
2024-06-27Have testBlockValidity hold cs_main instead of callerSjors Provoost
The goal of interfaces is to eventually run in their own process, so we can't use EXCLUSIVE_LOCKS_REQUIRED in their declaration. However TestBlockValidaty will crash (in its call to ConnectBlock) if the tip changes from under the proposed block. Have the testBlockValidity implementation hold the lock instead, and non-fatally check for this condition.
2024-06-27refactor: use CHECK_NONFATAL to avoid single-use symbolSjors Provoost
generated by cgit v1.2.3 (git 2.26.2) at 2025-03-14 21:03:15 +0000