💬 hebasto commented on pull request "coins: make sure PoolAllocator uses the correct alignment":
(https://github.com/bitcoin/bitcoin/pull/28913#issuecomment-1817924791)
> The pattern of `alignof(void*)` was used in 3 places in #25325 (also bench and tests) - looks like those other spots could be changed as well? (just a superficial observation, I didn't review that PR)

Test doesn't fail now. Maybe replace `int` with `int64`?

💬 martinus commented on pull request "coins: make sure PoolAllocator uses the correct alignment":
(https://github.com/bitcoin/bitcoin/pull/28913#issuecomment-1817927114)
> The pattern of alignof(void*) was used in 3 places

Thanks, I forgot about these... I'll change the PoolAllocator to default to the alignment of the given type, that makes much more sense and reduces the code.

> Test doesn't fail now. Maybe replace int with int64_t?
Good idea, I'll change that too

💬 hebasto commented on pull request "coins: make sure PoolAllocator uses the correct alignment":
(https://github.com/bitcoin/bitcoin/pull/28913#issuecomment-1817934135)
Concept ACK.

💬 fujicoin commented on issue "Signet mining is not possible when using descriptor wallet":
(https://github.com/bitcoin/bitcoin/issues/28911#issuecomment-1817941716)
@furszy
You can reproduce the phenomenon by following the steps below.

```
wget https://bitcoincore.org/bin/bitcoin-core-25.0/bitcoin-25.0-x86_64-linux-gnu.tar.gz
tar -xzvf bitcoin-25.0-x86_64-linux-gnu.tar.gz
cd bitcoin-25.0
mv bin/* .

git clone https://github.com/bitcoin/bitcoin.git
cd bitcoin
git checkout v25.0
cd ..
cp -r bitcoin/test/functional/test_framework .
cp bitcoin/contrib/signet/miner .

./bitcoind -regtest -daemon
./bitcoin-cli -regtest createwallet "test" false
...

💬 pinheadmz commented on pull request "coins: make sure PoolAllocator uses the correct alignment":
(https://github.com/bitcoin/bitcoin/pull/28913#issuecomment-1817951572)
concept ACK
running this branch on my RPi same as https://github.com/bitcoin/bitcoin/issues/28718#issuecomment-1816281195 will update

💬 sipa commented on pull request "coins: make sure PoolAllocator uses the correct alignment":
(https://github.com/bitcoin/bitcoin/pull/28913#issuecomment-1817953959)
utACK 486c2916a18b56d6011117077d906c1f8a81623f

💬 1440000bytes commented on pull request "wallet: Deniability API (Unilateral Transaction Meta-Privacy)":
(https://github.com/bitcoin/bitcoin/pull/27792#issuecomment-1817954781)
@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

💬 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
...