📝 theStack opened a pull request: "test: add coverage for sigop limit policy (`-bytespersigop` setting)"
(https://github.com/bitcoin/bitcoin/pull/27171)
This PR adds missing test coverage for the `-bytespersigop` option, which determines how pre-taproot signature operations (OP_CHECKSIG{VERIFY}, OP_CHECKMULTIGSIG{VERIFY}) affect fee handling calculations. The setting was introduced in PR #7081 for mitigating the [sigop spam attack](https://bitcointalk.org/index.php?topic=1166928.0); the initial implementation rejected txs exceeding the limit, but was changed in #8365 later to account for higher sizes in the mempool (i.e. exceeding the sigop limi
...
(https://github.com/bitcoin/bitcoin/pull/27171)
This PR adds missing test coverage for the `-bytespersigop` option, which determines how pre-taproot signature operations (OP_CHECKSIG{VERIFY}, OP_CHECKMULTIGSIG{VERIFY}) affect fee handling calculations. The setting was introduced in PR #7081 for mitigating the [sigop spam attack](https://bitcointalk.org/index.php?topic=1166928.0); the initial implementation rejected txs exceeding the limit, but was changed in #8365 later to account for higher sizes in the mempool (i.e. exceeding the sigop limi
...
💬 pablomartin4btc commented on pull request "Mask values on Transactions View":
(https://github.com/bitcoin-core/gui/pull/708#discussion_r1119564370)
Thanks @furszy, I took your advice on the decoupling the `dataChanged` signal and I've refactored the `updateDisplayUnit` `Q_SLOT` accordingly. As I was still thinking as you said, the call to `updateDisplayUnit` it's kind of a hack, so I've managed to add the connect on the `WalletView constructor` to trigger the `refreshAmounts` on the `setPrivacy`.
(https://github.com/bitcoin-core/gui/pull/708#discussion_r1119564370)
Thanks @furszy, I took your advice on the decoupling the `dataChanged` signal and I've refactored the `updateDisplayUnit` `Q_SLOT` accordingly. As I was still thinking as you said, the call to `updateDisplayUnit` it's kind of a hack, so I've managed to add the connect on the `WalletView constructor` to trigger the `refreshAmounts` on the `setPrivacy`.
💬 koga73 commented on issue "CGminer can't connect over RPC on 0.20.0":
(https://github.com/bitcoin/bitcoin/issues/19182#issuecomment-1447573050)
> after starting ckpool, on fresh install of bitcoind and ready synced with txindex=1 I got this lines on bitcoind, and ckpool tries and tries this :
>
> `[2021-01-19 08:47:53.442] CRITICAL: No bitcoinds active! [2021-01-19 08:47:55.446] JSON failed to decode GBT 0000000000000000000048840346957d513da4037b325689baa97f5acecabd90 0000000000000000000da8a10000000000000000000000000000000000000000 536870912 1611042475 170da8a1 (null) with errno 11: Resource temporarily unavailable [2021-01-19 08:47:
...
(https://github.com/bitcoin/bitcoin/issues/19182#issuecomment-1447573050)
> after starting ckpool, on fresh install of bitcoind and ready synced with txindex=1 I got this lines on bitcoind, and ckpool tries and tries this :
>
> `[2021-01-19 08:47:53.442] CRITICAL: No bitcoinds active! [2021-01-19 08:47:55.446] JSON failed to decode GBT 0000000000000000000048840346957d513da4037b325689baa97f5acecabd90 0000000000000000000da8a10000000000000000000000000000000000000000 536870912 1611042475 170da8a1 (null) with errno 11: Resource temporarily unavailable [2021-01-19 08:47:
...
💬 TheCharlatan commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#issuecomment-1447650893)
Updated b640db3c5fbe49af58dbc9087a96cbb493fa9574 -> b640db3c5fbe49af58dbc9087a96cbb493fa9574 ([removeBlockstorageArgs_2](https://github.com/TheCharlatan/bitcoin/tree/removeBlockstorageArgs_2) -> [removeBlockstorageArgs_3](https://github.com/TheCharlatan/bitcoin/tree/removeBlockstorageArgs_3), [compare](https://github.com/TheCharlatan/bitcoin/compare/removeBlockstorageArgs_2..removeBlockstorageArgs_3)) with suggested typo fix.
(https://github.com/bitcoin/bitcoin/pull/27125#issuecomment-1447650893)
Updated b640db3c5fbe49af58dbc9087a96cbb493fa9574 -> b640db3c5fbe49af58dbc9087a96cbb493fa9574 ([removeBlockstorageArgs_2](https://github.com/TheCharlatan/bitcoin/tree/removeBlockstorageArgs_2) -> [removeBlockstorageArgs_3](https://github.com/TheCharlatan/bitcoin/tree/removeBlockstorageArgs_3), [compare](https://github.com/TheCharlatan/bitcoin/compare/removeBlockstorageArgs_2..removeBlockstorageArgs_3)) with suggested typo fix.
💬 martinus commented on pull request "tracing: lock contention analysis":
(https://github.com/bitcoin/bitcoin/pull/25081#issuecomment-1447740069)
Hi, I lost interest / don't have time for this unfortunately. You can close it and mark it as up-for-grabs.
(https://github.com/bitcoin/bitcoin/pull/25081#issuecomment-1447740069)
Hi, I lost interest / don't have time for this unfortunately. You can close it and mark it as up-for-grabs.
✅ fanquake closed a pull request: "tracing: lock contention analysis"
(https://github.com/bitcoin/bitcoin/pull/25081)
(https://github.com/bitcoin/bitcoin/pull/25081)
💬 fanquake commented on pull request "refactor: Stop using gArgs global in system.cpp":
(https://github.com/bitcoin/bitcoin/pull/27170#issuecomment-1447846541)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/27170#issuecomment-1447846541)
Concept ACK
💬 TheCharlatan commented on pull request "Deduplicate bitcoind and bitcoin-qt init code":
(https://github.com/bitcoin/bitcoin/pull/27150#discussion_r1119622390)
nit: s/to whether/whether to/
(https://github.com/bitcoin/bitcoin/pull/27150#discussion_r1119622390)
nit: s/to whether/whether to/
💬 TheCharlatan commented on pull request "Deduplicate bitcoind and bitcoin-qt init code":
(https://github.com/bitcoin/bitcoin/pull/27150#discussion_r1119655981)
More a comment in case anybody else stumbles over this than a change request, but I'm surprised this is done in `bitcoind` and not in `bitcoin-qt`. Indeed I can start bitcoin-qt with any number of positional arguments, e.g. `./qt/bitcoin-qt h j k`, but not so `bitcoind`. The historical [context](https://github.com/bitcoin/bitcoin/pull/10447) is that this is a leftover from when `bitcoind` was also used as a command line client.
(https://github.com/bitcoin/bitcoin/pull/27150#discussion_r1119655981)
More a comment in case anybody else stumbles over this than a change request, but I'm surprised this is done in `bitcoind` and not in `bitcoin-qt`. Indeed I can start bitcoin-qt with any number of positional arguments, e.g. `./qt/bitcoin-qt h j k`, but not so `bitcoind`. The historical [context](https://github.com/bitcoin/bitcoin/pull/10447) is that this is a leftover from when `bitcoind` was also used as a command line client.
👍 darosior approved a pull request: "Make miniscript_{stable,smart} fuzzers avoid too large scripts"
(https://github.com/bitcoin/bitcoin/pull/27165)
ACK b09b7a1f94885feed5d44a3858543536e3659287
Elegant fix and improvement. Just two non-blocking comments.
(https://github.com/bitcoin/bitcoin/pull/27165)
ACK b09b7a1f94885feed5d44a3858543536e3659287
Elegant fix and improvement. Just two non-blocking comments.
💬 darosior commented on pull request "Make miniscript_{stable,smart} fuzzers avoid too large scripts":
(https://github.com/bitcoin/bitcoin/pull/27165#discussion_r1119807797)
Maybe worth pointing to the explanatory comment in `Parse()`? I think the "borrowing" logic could be a little unclear to a reader with less context.
(https://github.com/bitcoin/bitcoin/pull/27165#discussion_r1119807797)
Maybe worth pointing to the explanatory comment in `Parse()`? I think the "borrowing" logic could be a little unclear to a reader with less context.
💬 darosior commented on pull request "Make miniscript_{stable,smart} fuzzers avoid too large scripts":
(https://github.com/bitcoin/bitcoin/pull/27165#discussion_r1119792156)
You can directly pass the number of sub fragments to `ComputeScriptLen` instead of passing `0` and then adding it.
```patch
diff --git a/src/test/fuzz/miniscript.cpp b/src/test/fuzz/miniscript.cpp
index 90d752b898..490fac18ce 100644
--- a/src/test/fuzz/miniscript.cpp
+++ b/src/test/fuzz/miniscript.cpp
@@ -781,7 +781,7 @@ NodeRef GenNode(F ConsumeNode, Type root_type, bool strict_valid = false) {
auto node_info = ConsumeNode(type_needed);
if (!node_info) return {
...
(https://github.com/bitcoin/bitcoin/pull/27165#discussion_r1119792156)
You can directly pass the number of sub fragments to `ComputeScriptLen` instead of passing `0` and then adding it.
```patch
diff --git a/src/test/fuzz/miniscript.cpp b/src/test/fuzz/miniscript.cpp
index 90d752b898..490fac18ce 100644
--- a/src/test/fuzz/miniscript.cpp
+++ b/src/test/fuzz/miniscript.cpp
@@ -781,7 +781,7 @@ NodeRef GenNode(F ConsumeNode, Type root_type, bool strict_valid = false) {
auto node_info = ConsumeNode(type_needed);
if (!node_info) return {
...
💬 hebasto commented on pull request "refactor: Stop using gArgs global in system.cpp":
(https://github.com/bitcoin/bitcoin/pull/27170#issuecomment-1447896322)
Concept ACK.
(https://github.com/bitcoin/bitcoin/pull/27170#issuecomment-1447896322)
Concept ACK.
📝 fanquake opened a pull request: "guix: switch to some `minimal` versions of packages in our manifest"
(https://github.com/bitcoin/bitcoin/pull/27172)
Minimal versions of the same packages, that should still be sufficient for our use:
> (define-public bash-minimal
;; A stripped-down Bash for non-interactive use.
> (define-public coreutils-minimal
;; Coreutils without its optional dependencies.
(https://github.com/bitcoin/bitcoin/pull/27172)
Minimal versions of the same packages, that should still be sufficient for our use:
> (define-public bash-minimal
;; A stripped-down Bash for non-interactive use.
> (define-public coreutils-minimal
;; Coreutils without its optional dependencies.
📝 fanquake converted_to_draft a pull request: "RPC/Wallet: Convert walletprocesspsbt to use options parameter"
(https://github.com/bitcoin/bitcoin/pull/24963)
null
(https://github.com/bitcoin/bitcoin/pull/24963)
null
💬 fanquake commented on pull request "RPC/Wallet: Convert walletprocesspsbt to use options parameter":
(https://github.com/bitcoin/bitcoin/pull/24963#issuecomment-1447931166)
Drafted for now. Needs a PR description and rebasing to fix the merge conflict:
```bash
wallet/rpc/spend.cpp:1541:75: error: no member named 'OMITTED_NAMED_ARG' in 'RPCArg::Optional'
{"options|sign", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "",
~~~~~~~~~~~~~~~~~~^
wallet/rpc/spend.cpp:1600:9: error: use of undeclared identifier 'RPCTypeCheckArgument'
RPCTypeCheckArgument(options, UniValue::VO
...
(https://github.com/bitcoin/bitcoin/pull/24963#issuecomment-1447931166)
Drafted for now. Needs a PR description and rebasing to fix the merge conflict:
```bash
wallet/rpc/spend.cpp:1541:75: error: no member named 'OMITTED_NAMED_ARG' in 'RPCArg::Optional'
{"options|sign", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "",
~~~~~~~~~~~~~~~~~~^
wallet/rpc/spend.cpp:1600:9: error: use of undeclared identifier 'RPCTypeCheckArgument'
RPCTypeCheckArgument(options, UniValue::VO
...
💬 hebasto commented on pull request "guix: switch to some `minimal` versions of packages in our manifest":
(https://github.com/bitcoin/bitcoin/pull/27172#issuecomment-1447934046)
Concept ACK.
(https://github.com/bitcoin/bitcoin/pull/27172#issuecomment-1447934046)
Concept ACK.
🚀 fanquake merged a pull request: "depends: harden libevent"
(https://github.com/bitcoin/bitcoin/pull/27118)
(https://github.com/bitcoin/bitcoin/pull/27118)
💬 fanquake commented on pull request "security-check: test for `_FORTIFY_SOURCE` usage in release binaries":
(https://github.com/bitcoin/bitcoin/pull/27038#issuecomment-1447937422)
Rebased post #27118.
(https://github.com/bitcoin/bitcoin/pull/27038#issuecomment-1447937422)
Rebased post #27118.
💬 TheCharlatan commented on pull request "guix: switch to some `minimal` versions of packages in our manifest":
(https://github.com/bitcoin/bitcoin/pull/27172#issuecomment-1447942274)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/27172#issuecomment-1447942274)
Concept ACK
📝 fanquake converted_to_draft a pull request: "RPC/Wallet: Add "use_txids" to output of getaddressinfo"
(https://github.com/bitcoin/bitcoin/pull/22693)
(Non-GUI part of #15987, but without the bloom stuff)
(https://github.com/bitcoin/bitcoin/pull/22693)
(Non-GUI part of #15987, but without the bloom stuff)