💬 1440000bytes commented on pull request "wallet: Deniability API (Unilateral Transaction Meta-Privacy)":
(https://github.com/bitcoin/bitcoin/pull/27792#issuecomment-1817956896)
> @denavila I am okay with this merged for testnet or signet alleast
>
> We want to improve privacy and see results
>
> Thats not how things work here

Please join to @luke-jr in https://github.com/bitcoinknots/bitcoin

💬 furszy commented on issue "Signet mining is not possible when using descriptor wallet":
(https://github.com/bitcoin/bitcoin/issues/28911#issuecomment-1817957086)
Ok, yes. Thats what I imagined.

Your challenge `512103176706ed990b936e2348392c08cfc9b7a8550d88b2b4cde4175bfb3b8af6398e51ae`, is a 1-of-1 multisig of your pubkey.
This script is not watched by your wallet by default, is a consensus parameter. The wallet migration process will not create the matching multisig descriptor.

You either can:
1) Import the multisig redeem script on the legacy wallet (pre-migration).
2) Import the multisig descriptor on the descriptor wallet (post-migration).

...

💬 TheCharlatan commented on pull request "lint: Report all lint errors instead of early exit":
(https://github.com/bitcoin/bitcoin/pull/28862#discussion_r1398492522)
Maybe just add a "failure in subtree lint" (also for the others)?

💬 TheCharlatan commented on pull request "lint: Report all lint errors instead of early exit":
(https://github.com/bitcoin/bitcoin/pull/28862#discussion_r1398492247)
Without state variables or `if`s:
``` Rust
[
"src/crypto/ctaes",
"src/secp256k1",
"src/minisketch",
"src/leveldb",
"src/crc32c",
].iter().all(
|&subtree| Command::new("test/lint/git-subtree-check.sh")
.arg(subtree)
.status()
.expect("command_error")
.success()
).then(|| ()).ok_or("".to_string())
```

💬 fujicoin commented on issue "Signet mining is not possible when using descriptor wallet":
(https://github.com/bitcoin/bitcoin/issues/28911#issuecomment-1818016414)
@furszy Thank you for your accurate advice.
I confirmed that signet mining can be executed successfully using the second method (Import the multisig descriptor on the descriptor wallet).

💬 pinheadmz commented on pull request "coins: make sure PoolAllocator uses the correct alignment":
(https://github.com/bitcoin/bitcoin/pull/28913#issuecomment-1818044888)
Up to height 376554 now which is way further than I crashed at on 26rc2. htop says RAM is only about 890MB used. Are there any RPCs I should check?

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#issuecomment-1818210163)
> But it seems confusing to fail with a stack trace / uncaught exception in this case ("Fatal error: protocol.data_received() call failed.") when this is actually expected to happen.


<details>
<summary> is this the test log you observed? </summary>
<br>

```
2023-11-18T13:52:32.976000Z TestFramework (INFO): PRNG seed is: 8699047777059301467
2023-11-18T13:52:32.977000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_70zlmcgr
2023-11-18T13:52:33.535000Z TestFra
...

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1398651836)
you're right! done.

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1398651878)
done.

💬 ajtowns commented on pull request "refactor: P2P transport without serialize version and type":
(https://github.com/bitcoin/bitcoin/pull/28892#discussion_r1398688765)
Calling `NetMsg::Make` within `PushMessage` seems unnecessary; just add an extra `PeerManagerImpl` helper:

```c++
template <typename... Args>
void PushMessage(CNode& node, std::string msg_type, Args&&... args) const
{
PushMessage(node, NetMsg::Make(std::move(msg_type), args...));
}
```

cf https://github.com/ajtowns/bitcoin/commit/0fe129b1b437d39221cef843f969f1adc31b543e

✅ fujicoin closed an issue: "Signet mining is not possible when using descriptor wallet"
(https://github.com/bitcoin/bitcoin/issues/28911)

💬 martinus commented on issue "RAM usage regression in 26.x and master on ARM 32-bit":
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1818343244)
> [...] try to make sure fallback allocations get counted too in master.

Related PRs: #27748 tries to do exactly this, and #28531 wants to improve MallocUsage estimation.

💬 martinus commented on pull request "util: generalize accounting of system-allocated memory in pool resource":
(https://github.com/bitcoin/bitcoin/pull/27748#issuecomment-1818347616)
@LarryRuane are you still working on this? I think this shouldn't be blocked by #28531.

📝 TaiseiLuette opened a pull request: "Create staging tree"
(https://github.com/bitcoin/bitcoin/pull/28914)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

✅ TaiseiLuette closed a pull request: "Create staging tree"
(https://github.com/bitcoin/bitcoin/pull/28914)

💬 maflcko commented on pull request "refactor: P2P transport without serialize version and type":
(https://github.com/bitcoin/bitcoin/pull/28892#discussion_r1398744244)
Correct, but I wasn't sure if reviewers preferred the slightly more verbose way to clarify that serialization is involved on every call.

💬 martinus commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#issuecomment-1818440265)
I can reproduce the same results with glibc 2.31 and 2.38, in 32bit and 64bit. I don't think it needs to be perfect on all platforms, I think your new calculation is fine.

I played a bit with a reproducer, here is mine: https://godbolt.org/z/zqhzxoobx I refactored your `MallocUsage` formula a bit, but it's basically the same

💬 maflcko commented on pull request "lint: Report all lint errors instead of early exit":
(https://github.com/bitcoin/bitcoin/pull/28862#discussion_r1398875960)
Are you sure? `all()` short circuits (https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.all), but the goal of this pull is to report all lint errors, not only the first one.

💬 maflcko commented on pull request "lint: Report all lint errors instead of early exit":
(https://github.com/bitcoin/bitcoin/pull/28862#discussion_r1398877832)
Also, style-wise, some people prefer `for` over functional programming when the predicate has side effects. Happy to push what you prefer, but I wanted to mention that this was written on purpose in this way.

💬 maflcko commented on pull request "lint: Report all lint errors instead of early exit":
(https://github.com/bitcoin/bitcoin/pull/28862#discussion_r1398881891)
I think this is already printed. For me:

```
...
^---- Failure generated from subtree check!
...