💬 l0rinc commented on pull request "Benchmark Chainstate::ConnectBlock duration":
(https://github.com/bitcoin/bitcoin/pull/31689#discussion_r1954428945)
Seems unused now

💬 l0rinc commented on pull request "Benchmark Chainstate::ConnectBlock duration":
(https://github.com/bitcoin/bitcoin/pull/31689#discussion_r1954439106)
We could avoid copying here as well:
```suggestion
const auto& test_block{CreateTestBlock(test_setup, keys, outputs)};
```

💬 l0rinc commented on pull request "Benchmark Chainstate::ConnectBlock duration":
(https://github.com/bitcoin/bitcoin/pull/31689#discussion_r1954436295)
```suggestion
// Doing this in a separate block excludes the validation of its inputs from the benchmark
```

💬 laanwj commented on pull request "net: reduce CAddress usage to CService or CNetAddr":
(https://github.com/bitcoin/bitcoin/pull/31854#issuecomment-2656522537)
Concept ACK

📝 fanquake opened a pull request: "depends: avoid an unset `CMAKE_OBJDUMP`"
(https://github.com/bitcoin/bitcoin/pull/31857)
Similar to #31840, currently our Linux toolchain file contains:
```bash
set(CMAKE_AR "aarch64-linux-gnu-ar")
set(CMAKE_RANLIB "aarch64-linux-gnu-ranlib")
set(CMAKE_STRIP "aarch64-linux-gnu-strip")
set(CMAKE_OBJCOPY "aarch64-linux-gnu-objcopy")
set(CMAKE_OBJDUMP "")
```

`objdump` is currently only used for the macOS cross build, where it's `llvm-objdump`, but we should be consistent in producing a toolchain file that points to actual tools, rather than leaving variables unset.

💬 ryanofsky commented on pull request "logging: Ensure -debug=0/none behaves consistently with -nodebug":
(https://github.com/bitcoin/bitcoin/pull/31767#discussion_r1954464935)
> Nit in [a8fedb3](https://github.com/bitcoin/bitcoin/commit/a8fedb36a71ac193fc158284b14d4eb474e5ab62): Would be nice to explain why this is done: Something like: `// GetArgs disregards any debugging categories appearing before -nodebug, so do the same for -debug=0/none`

Could be good to add a reason for this, but the suggested reason is pretty obscure one. I think the general reason for doing this is that later arguments are supposed to take precedence over earlier ones, so when -debug=none
...

💬 l0rinc commented on issue "doc/zmq: Note about endianness does not match reality":
(https://github.com/bitcoin/bitcoin/issues/31856#issuecomment-2656552688)
For reference, @hodlinator fixed a few of these in https://github.com/bitcoin/bitcoin/pull/30526 and @ryanofsky a few more in https://github.com/bitcoin/bitcoin/pull/31596.

💬 0xB10C commented on pull request "test, tracing: don't use problematic `bpf_usdt_readarg_p()`":
(https://github.com/bitcoin/bitcoin/pull/31848#issuecomment-2656566483)
> So if I understand the change correctly, the new (documented/recommended) behaviour is to always first read_arg() which gets the memory location of the data, then read_user{_str}() which then actually performs the copy from user memory to the BPF memory space.

Correct. Here's an example from the `mempool:added` tracepoint test.

- (1) initialize a null-pointer `phash`
- (2) read the first tracepoint argument (a user-space address) into the `phash` pointer
- (3) copy the value behind the
...

💬 hodlinator commented on pull request "test: check `scanning` field from `getwalletinfo`":
(https://github.com/bitcoin/bitcoin/pull/31768#discussion_r1954481759)
nit: Seems a bit heavy-handed to use exceptions, even if it's following the pattern of code above?
```suggestion
scanning = wallet_info.get("scanning")
if scanning:
assert_greater_than(scanning["duration"], 0)
```
Could also break out `encryped_cli = self.nodes[0].cli("-rpcwallet=encrypted_wallet")` for the entire block now that we are using it for a 4th time, if you re-touch.

💬 maflcko commented on pull request "logging: Ensure -debug=0/none behaves consistently with -nodebug":
(https://github.com/bitcoin/bitcoin/pull/31767#discussion_r1954487720)
Thx for confirming. It would be nice to remove it here, so that it doesn't have to be touched (and reviewed) again in another pull request. But it was just a nit and anything is fine.

💬 InnDe commented on issue "Wallet passpharse":
(https://github.com/bitcoin/bitcoin/issues/31852#issuecomment-2656583674)
> > i had the same problem, even in new test wallets I’ve created with same passpharse, the same problem
>
> Does the problem on a freshly created wallet happen with any passphrase, or only one specific one?

The one specific i described it in my question above sir

👍 l0rinc approved a pull request: "Benchmark Chainstate::ConnectBlock duration"
(https://github.com/bitcoin/bitcoin/pull/31689#pullrequestreview-2614976099)
ACK 7edaf8b64cb2d59ada22042fee62a417e52368b8

I ran the benchmarks and tests locally.
I need a crypto expert to validate them conceptually as well, but I'm fine with the benchmarking code now.

(before merging please fix the typo in the PR description)

💬 maflcko commented on issue "Wallet passpharse":
(https://github.com/bitcoin/bitcoin/issues/31852#issuecomment-2656602446)
I can't reproduce with `walletpassphrasechange '(space “thekey” \ \ \ \ \ \ \ \ \ \ \ \ )
“ \ \ \ \ \ \ \ \ \ \ \ \ )”' 'new_'` (it passes fine for me locally) on a freshly created wallet.

Please provide full (and exact) steps to reproduce on a freshly created wallet.

💬 InnDe commented on issue "Wallet passpharse":
(https://github.com/bitcoin/bitcoin/issues/31852#issuecomment-2656607996)
Ok i will, then i will provide screen recorded video
On Thu, 13 Feb 2025 at 16:28 maflcko ***@***.***> wrote:

> I can't reproduce with walletpassphrasechange '(space “thekey” \ \ \ \ \
> \ \ \ \ \ \ \ ) “ \ \ \ \ \ \ \ \ \ \ \ \ )”' 'new_' (it passes fine for
> me locally) on a freshly created wallet.
>
> Please provide full (and exact) steps to reproduce on a freshly created
> wallet.
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/bitcoin/bitcoin/issu
...

👍 hebasto approved a pull request: "depends: avoid an unset `CMAKE_OBJDUMP`"
(https://github.com/bitcoin/bitcoin/pull/31857#pullrequestreview-2615016680)
ACK 2434aeab62ba07c5380112838f3600b3dbbceef2.

💬 maflcko commented on issue "Wallet passpharse":
(https://github.com/bitcoin/bitcoin/issues/31852#issuecomment-2656625024)
Thx, but it should be sufficient to just copy-paste the commands (and results) into a comment here directly. You can use <code>`</code> for quoting.

💬 ryanofsky commented on pull request "logging: Ensure -debug=0/none behaves consistently with -nodebug":
(https://github.com/bitcoin/bitcoin/pull/31767#discussion_r1954520666)
> Thx for confirming. It would be nice to remove it here, so that it doesn't have to be touched (and reviewed) again in another pull request.

Yep, #30529 does have 2 acks though so getting close

🚀 ryanofsky merged a pull request: "logging: Ensure -debug=0/none behaves consistently with -nodebug"
(https://github.com/bitcoin/bitcoin/pull/31767)

💬 Sjors commented on pull request "Have createNewBlock() wait for tip, make rpc handle shutdown during long poll and wait methods":
(https://github.com/bitcoin/bitcoin/pull/31785#discussion_r1954529064)
I don't think we should drop the default, because `MillisecondsDouble::max()` is tedious. It seems fine that the interface does it internally, which is simpler than the other two solutions.

💬 InnDe commented on issue "Wallet passpharse":
(https://github.com/bitcoin/bitcoin/issues/31852#issuecomment-2656648103)
Ok, I will do that.
command: walletpassphrase ' \ \ \ \ \ \ \ \ \ \ \ \' 800
result:
result is absolutely nothing

On Thu, 13 Feb 2025 at 16:37, maflcko ***@***.***> wrote:

> Thx, but it should be sufficient to just copy-paste the commands (and
> results) into a comment here directly. You can use ` for quoting.
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/bitcoin/bitcoin/issues/31852#issuecomment-2656625024>,
> or unsubscribe
> <https://github.com/
...