💬 hebasto commented on pull request "Scale network graph based on time interval":
(https://github.com/bitcoin-core/gui/pull/539#issuecomment-1463678070)
> How do I label it as up for grabs?

Done.

💬 willcl-ark commented on pull request "github: Switch to yaml issue templates":
(https://github.com/bitcoin/bitcoin/pull/27025#discussion_r1132270445)
Hmmm, for me as a non admin this repo now looks like:


The green `View Policy` button takes me to a page which includes "Policy" and "Advisories", defaulting to showing the security policy `SECURITY.md`.

The row below that, "Bitcoin Core Security Policy" with the non-green `Open` button links me straight to `SECURITY.md`, which I agree is in theory somewhat redundant. If there i
...

💬 willcl-ark commented on issue "[RFC] Dealing with RPCs that have a lot of positional options":
(https://github.com/bitcoin/bitcoin/issues/22575#issuecomment-1463691588)
Can this be closed now that we have https://github.com/bitcoin/bitcoin/pull/19762 ? Or are we leaving it open to remember to address [this](https://github.com/bitcoin/bitcoin/issues/22575#issuecomment-889252965) comment:

> I think another important thing our RPC server is missing is the ability to require option-style parameters to be passed by name, and to disallow passing them by position.

💬 MarcoFalke commented on issue "[RFC] Dealing with RPCs that have a lot of positional options":
(https://github.com/bitcoin/bitcoin/issues/22575#issuecomment-1463698107)



> Can this be closed now that we have #19762 ? Or are we leaving it open to remember to address [this](https://github.com/bitcoin/bitcoin/issues/22575#issuecomment-889252965) comment:
>
> > I think another important thing our RPC server is missing is the ability to require option-style parameters to be passed by name, and to disallow passing them by position.

That is implemented in #26485, no?

💬 willcl-ark commented on issue "[RFC] Dealing with RPCs that have a lot of positional options":
(https://github.com/bitcoin/bitcoin/issues/22575#issuecomment-1463707740)
Oh yea, looks like it. Cool!

💬 MarcoFalke commented on pull request "assumeutxo: net_processing changes":
(https://github.com/bitcoin/bitcoin/pull/24008#discussion_r1132286555)
nit (feel free to ignore): What about passing a `const&` ref here? Otherwise someone could pass in a `nullptr` and run into UB at runtime?

💬 MarcoFalke commented on pull request "assumeutxo: net_processing changes":
(https://github.com/bitcoin/bitcoin/pull/24008#discussion_r1132291848)
nit: No need to call `get` if you call `*` later on. Also, can inline the assert:
```suggestion

return *Assert(m_ibd_chainstate);
```

👍 brunoerg approved a pull request: "Add test and docs for getblockfrompeer with pruning"
(https://github.com/bitcoin/bitcoin/pull/23813)
re-ACK fe329dc936d1e02da406345e4223e11d1fa6fb38

👍 MarcoFalke approved a pull request: "test: add coverage for sigop limit policy (`-bytespersigop` setting)"
(https://github.com/bitcoin/bitcoin/pull/27171)
nice ACK 89cd20cbedbba344bab92dd1d71dac9c84320a70 📁

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: nice ACK 89cd20cbedbba34
...

💬 MarcoFalke commented on pull request "test: add coverage for sigop limit policy (`-bytespersigop` setting)":
(https://github.com/bitcoin/bitcoin/pull/27171#discussion_r1132304879)
for reference, usually `fAccurate` is set for witness scripts sigop counts, but omitting the k and n in the CMS imitates `fAccurate=false`.

💬 MarcoFalke commented on pull request "test: Use self.wait_until over wait_until_helper":
(https://github.com/bitcoin/bitcoin/pull/27226#issuecomment-1463740089)
review ACK fe329dc936d1e02da406345e4223e11d1fa6fb38 🍉

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK fe329dc936d1
...

💬 MarcoFalke commented on pull request "Add test and docs for getblockfrompeer with pruning":
(https://github.com/bitcoin/bitcoin/pull/23813#issuecomment-1463740629)
review ACK fe329dc936d1e02da406345e4223e11d1fa6fb38 🍉

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK fe329dc936d1
...

👋 TheCharlatan's pull request is ready for review: "refactor: Split logging utilities from system.h"
(https://github.com/bitcoin/bitcoin/pull/27238)

💬 MarcoFalke commented on pull request "test: fix race condition in encrypted wallet rescan tests":
(https://github.com/bitcoin/bitcoin/pull/27199#discussion_r1132331631)
You'll have to do this sync on `t.start`, *before* `walletlock`, not *after* `walletlock`, no?

Otherwise this doesn't achieve what you want and will fail anyway:

```
AssertionError: [node 0] Expected messages "['Rescan started from block 0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206... (slow variant inspecting all blocks)']" does not partially match log:
```

To test:

```diff
diff --git a/test/functional/wallet_importdescriptors.py b/test/functional/wallet_import
...

💬 MarcoFalke commented on issue "Issue with `wallet_importdescriptors.py --descriptors` under valgrind":
(https://github.com/bitcoin/bitcoin/issues/27229#issuecomment-1463760044)
Maybe re-try after https://github.com/bitcoin/bitcoin/pull/27199 ?

👍 TheCharlatan approved a pull request: "refactor: Consistenly use context args over gArgs in node/interfaces"
(https://github.com/bitcoin/bitcoin/pull/27239)
ACK [facbb44](https://github.com/bitcoin/bitcoin/pull/27239/commits/facbb444bf5aea2bbaa4a4246a8a2c661d9bf314)

This has been confusing me recently, thank you for cleaning this up. I reproduced the patchset and did not find similar mixed uses of context and gArgs in other files.

💬 MarcoFalke commented on pull request "doc: Show how less noisy clang-tidy output can be achieved":
(https://github.com/bitcoin/bitcoin/pull/27205#discussion_r1132336428)
Why not just modify the above command to include this? (And then mention that `--config` can be omitted, if someone really wants to)

I'd suspect that you run into a wall of errors anyway without `--config`

💬 Sjors commented on issue "ops: Enable DNSSEC on all Bitcoin DNS Seed domain names":
(https://github.com/bitcoin/bitcoin/issues/19714#issuecomment-1463768303)
I just noticed @Emzy's script and PR on the seeder repo. Will look into it soon(tm).

💬 Sjors commented on pull request "assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/15606#discussion_r1132363873)
Optimistically bumping the block, but also making sure I don't keep putting the wrong numbers here:

```cpp
/*height=*/780000,
{AssumeutxoHash{uint256S(/*txoutset_hash=*/"0x97d2989ab4c1b2cfb4eaa37788aeea08a3a6757eabd94ae9b7cdde07e79381da")},
/*nchaintx=*/812391117},
```

Maybe also add a string initialiser to `AssumeutxoHash` to avoid the extra `uint256S`.

🚀 fanquake merged a pull request: "Add test and docs for getblockfrompeer with pruning"
(https://github.com/bitcoin/bitcoin/pull/23813)