aboutsummaryrefslogtreecommitdiff
path: root/src/rpc
AgeCommit message (Collapse)Author
2024-09-03Merge bitcoin/bitcoin#26619: log: expand BCLog::LogFlags (categories) to 64 bitsAva Chow
b31a0cd0378184b2b9eb8f4bd3120cbd32c62005 log: expand BCLog::LogFlags (categories) to 64 bits (Larry Ruane) Pull request description: Increase the maximum number of logging categories from 32 to 64. We're currently using 29 of the 32 available logging categories (there are only 3 remaining). It would be good to increase the limit soon; the fourth PR to be merged that adds a new logging category will be blocked until something like this is done. This PR also adds a `TEST` category that uses the new range (`1ULL << 63`) in case there's a hidden assumption somewhere that the `BCLog::LogFlags` type is 32 bits. (Also added a test for this test category.) It also provides an example showing that the expression must be `1ULL << <shift>` for shift value 31 and beyond. ACKs for top commit: achow101: ACK b31a0cd0378184b2b9eb8f4bd3120cbd32c62005 vasild: ACK b31a0cd0378184b2b9eb8f4bd3120cbd32c62005 ryanofsky: Code review ACK b31a0cd0378184b2b9eb8f4bd3120cbd32c62005, just dropping mask_bit constant since last review. I still think theStack: Code-review ACK b31a0cd0378184b2b9eb8f4bd3120cbd32c62005 Tree-SHA512: de422dbeb479848d370aed42d415f42461457ab0eda62b245dc7ff9f0e111626e7d4c0d62ff13082ec664d05fbb0db04c71eb4b6f22eb8f19198826a67c4035e
2024-09-03Merge bitcoin/bitcoin#30742: kernel: Use spans instead of vectors for ↵Ava Chow
passing block headers to validation functions a2955f09792b6232f3a45aa44a498b466279a8b7 validation: Use span for ImportBlocks paths (TheCharlatan) 20515ea3f5bd426f6e3746cf5cddd2324dacae31 validation: Use span for CalculateClaimedHeadersWork (TheCharlatan) 52575e96e72a0402c448f86728b2e84836b1e817 validation: Use span for ProcessNewBlockHeaders (TheCharlatan) Pull request description: Makes it friendlier for potential future users of the kernel library if they do not store the headers in a std::vector, but can guarantee contiguous memory. Take this opportunity to also change the argument of ImportBlocks previously taking a `std::vector` to a `std::span`. ACKs for top commit: stickies-v: re-ACK a2955f09792b6232f3a45aa44a498b466279a8b7 - no changes except further walking the ~file~ path of modernizing variable names. maflcko: ACK a2955f09792b6232f3a45aa44a498b466279a8b7 🕑 achow101: ACK a2955f09792b6232f3a45aa44a498b466279a8b7 danielabrozzoni: ACK a2955f09792b6232f3a45aa44a498b466279a8b7 Tree-SHA512: 8b07f4ad26e270b65600d1968cd78847b85caca5bfbb83fd9860389f26656b1d9a40b85e0990339f50403d18cedcd2456990054f3b8b0bedce943e50222d2709
2024-09-03Merge bitcoin/bitcoin#29553: assumeutxo: Add dumptxoutset height param, ↵Ava Chow
remove shell scripts 94b0adcc371540732453d70309c4083d4bd9cd6b rpc, refactor: Prevent potential race conditions in dumptxoutset (Fabian Jahr) e868a6e070a91c00555e72181f9b14bbf0373fdc doc: Improve assumeutxo guide and add more docs/comments (Fabian Jahr) b29c21fc92dcc3da95bd032ba41675a8b9a0a24b assumeutxo: Remove devtools/utxo_snapshot.sh (Fabian Jahr) 20a1c77aa7dec2449071187a439d17f7aeaee648 contrib: Remove test_utxo_snapshots.sh (Fabian Jahr) 842685035244e151f4a10019af2dfe0563f11a82 test: Test for dumptxoutset at specific height (Fabian Jahr) 993cafe7e45ab0af1e862c7def3de688f47c0443 RPC: Add type parameter to dumptxoutset (Fabian Jahr) fccf4f91d21c351d742943d35476f53d40963b8b RPC: Extract ReconsiderBlock helper (Fabian Jahr) 446ce51c21cd2466cb12fa0166fd069d42b603bf RPC: Extract InvalidateBlock helper (Fabian Jahr) Pull request description: This adds a height parameter to the `dumptxoutset` RPC. This internalizes the workflow that was previously done by scripts: roll back the chain to the height we actually want the snapshot from, create the snapshot, roll forward to the real tip again. The nice thing about internalizing this functionality is that we can write tests for the code and it gives us more options to make the functionality robust. The shell scripts we have so far will be more cumbersome to maintain in the long run, especially since we will only notice later when we have broken them. I think it's safe to remove these `test_utxo_snapshots.sh` as well when we have this option in `dumptxoutset` because we have also added some good additional functional test coverage for this functionality. ACKs for top commit: Sjors: re-utACK 94b0adcc371540732453d70309c4083d4bd9cd6b achow101: ACK 94b0adcc371540732453d70309c4083d4bd9cd6b mzumsande: ACK 94b0adcc371540732453d70309c4083d4bd9cd6b pablomartin4btc: re-ACK 94b0adcc371540732453d70309c4083d4bd9cd6b Tree-SHA512: a4c9af5f687d1ca7bfb579a36f363882823386b5fa80c05de531b05a2782b5da6ff5baf3ada4bca8f32f63975d86f1948175abed9affe51fc958472b5f838dab
2024-09-01rpc, refactor: Prevent potential race conditions in dumptxoutsetFabian Jahr
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2024-09-01doc: Improve assumeutxo guide and add more docs/commentsFabian Jahr
Also fixes some outdated information in the remaining design doc.
2024-09-01RPC: Add type parameter to dumptxoutsetFabian Jahr
2024-09-01RPC: Extract ReconsiderBlock helperFabian Jahr
2024-09-01RPC: Extract InvalidateBlock helperFabian Jahr
2024-08-30validation: Use span for ProcessNewBlockHeadersTheCharlatan
Makes it friendlier for potential future users of the kernel library if they do not store the headers in a std::vector, but can guarantee contiguous memory.
2024-08-29scripted-diff: LogPrint -> LogDebugMarcoFalke
-BEGIN VERIFY SCRIPT- sed -i 's/\<LogPrint\>/LogDebug/g' $( git grep -l '\<LogPrint\>' -- ./contrib/ ./src/ ./test/ ':(exclude)src/logging.h' ) -END VERIFY SCRIPT-
2024-08-28Merge bitcoin/bitcoin#22838: descriptors: Be able to specify change and ↵glozow
receiving in a single descriptor string a0abcbd3822bd17a1d73c42ccd5b040a150b0501 doc: Mention multipath specifier (Ava Chow) 0019f61fc546b4d5f42eb4086f42560863fe0efb tests: Test importing of multipath descriptors (Ava Chow) f97d5c137d605ac48f1122a836c9aa5f834957ba wallet, rpc: Allow importdescriptors to import multipath descriptors (Ava Chow) 32dcbca3fb918bc899a0637f876db31c3419aafd rpc: Allow importmulti to import multipath descriptors correctly (Ava Chow) 64dfe3ce4bed9ac168d0b08def8af7485db94ef1 wallet: Move internal to be per key when importing (Ava Chow) 16922455253f47fae0466c4ec6c3adfadcfe9182 tests: Multipath descriptors for scantxoutset and deriveaddresses (Ava Chow) cddc0ba9a9dca3ca5873d768b3b504cdb2ab947b rpc: Have deriveaddresses derive receiving and change (Ava Chow) 360456cd221501fde3efe11bdba5c6d999dbb323 tests: Multipath descriptors for getdescriptorinfo (Ava Chow) a90eee444c965bbd7bcddf9656eca9cee14c3aec tests: Add unit tests for multipath descriptors (Ava Chow) 1bbf46e2dae4599d04c79aaacf7c5db00b2e707f descriptors: Change Parse to return vector of descriptors (Ava Chow) 0d640c6f02bc20e5c1be773443dd74d8806d953b descriptors: Have ParseKeypath handle multipath specifiers (Ava Chow) a5f39b103461a98689fd5d382e8da29037f55bea descriptors: Change ParseScript to return vector of descriptors (Ava Chow) 0d55deae157f4f8226b2419d55e7dc0dfb6e4aec descriptors: Add DescriptorImpl::Clone (Ava Chow) 7e86541f723d62c7ec6768f7f592c09ba2047d9e descriptors: Add PubkeyProvider::Clone (Ava Chow) Pull request description: It is convenient to have a descriptor which specifies both receiving and change addresses in a single string. However, as discussed in https://github.com/bitcoin/bitcoin/issues/17190#issuecomment-895515768, it is not feasible to use a generic multipath specification like BIP 88 due to combinatorial blow up and that it would result in unexpected descriptors. To resolve that problem, this PR proposes a targeted solution which allows only a single pair of 2 derivation indexes to be inserted in the place of a single derivation index. So instead of two descriptor `wpkh(xpub.../0/0/*)` and `wpkh(xpub.../0/1/*)` to represent receive and change addresses, this could be written as `wpkh(xpub.../0/<0;1>/*)`. The multipath specifier is of the form `<NUM;NUM>`. Each `NUM` can have its own hardened specifier, e.g. `<0;1h>` is valid. The multipath specifier can also only appear in one path index in the derivation path. This results in the parser returning two descriptors. The first descriptor uses the first `NUM` in all pairs present, and the second uses the second `NUM`. In our implementation, if a multipath descriptor is not provided, a pair is still returned, but the second element is just `nullptr`. The wallet will not output the multipath descriptors (yet). Furthermore, when a multipath descriptor is imported, it is expanded to the two descriptors and each imported on its own, with the second descriptor being implicitly for internal (change) addresses. There is no change to how the wallet stores or outputs descriptors (yet). Note that the path specifier is different from what was proposed. It uses angle brackets and the semicolon because these are unused characters available in the character set and I wanted to avoid conflicts with characters already in use in descriptors. Closes #17190 ACKs for top commit: darosior: re-ACK a0abcbd3822bd17a1d73c42ccd5b040a150b0501 mjdietzx: reACK a0abcbd3822bd17a1d73c42ccd5b040a150b0501 pythcoiner: reACK a0abcbd furszy: Code review ACK a0abcbd glozow: light code review ACK a0abcbd3822 Tree-SHA512: 84ea40b3fd1b762194acd021cae018c2f09b98e595f5e87de5c832c265cfe8a6d0bc4dae25785392fa90db0f6301ddf9aea787980a29c74f81d04b711ac446c2
2024-08-13log: expand BCLog::LogFlags (categories) to 64 bitsLarry Ruane
This will increase the maximum number of logging categories from 32 to 64.
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-08rpc: Have deriveaddresses derive receiving and changeAva Chow
When given a multipath descriptor, derive all of the descriptors. The derived addresses will be returned in an object consisting of multiple arrays. For compatibility, when given a single path descriptor, the addresses are provided in a single array as before.
2024-08-08descriptors: Change Parse to return vector of descriptorsAva Chow
When given a descriptor which contins a multipath derivation specifier, a vector of descriptors will be returned.
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.