📝 martinus opened a pull request: "coins: make sure PoolAllocator uses the correct alignment"
(https://github.com/bitcoin/bitcoin/pull/28913)
The class `CTxOut` has a member `CAmount` which is an int64_t, and on ARM 32bit int64_t are 8 byte aligned, which is larger than the pointer alignment of 4 bytes.

So for `CCoinsMap` to be able to use the pool, we need to use the alignment of the member instead of just `alignof(void*)`.

This fixes #28906 (first noted in https://github.com/bitcoin/bitcoin/issues/28718#issuecomment-1807197107)

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a
...

💬 hebasto commented on issue "26.0 RC Testing Guide Feedback":
(https://github.com/bitcoin/bitcoin/issues/28866#issuecomment-1817905395)
FWIW, the https://github.com/bitcoin/bitcoin/pull/25325 improves the UTXO cache performance noticeably. Might be tested with `bitcoind -reindex-chainstate`.

💬 martinus commented on issue "Impossible to run bitcoin on a Raspberry Pi 4 with 8GB, on Raspian 64 bit, which ships a 32 bit version of Docker (ARMHF instead of ARM64)":
(https://github.com/bitcoin/bitcoin/issues/28440#issuecomment-1817906128)
I'm pretty sure this issue is fixed in #28913 , because the logs here (`[...] cache=226.8MiB(31133550txo)`) indicate that the PoolAllocator is not used for the nodes, leading to incorrect cache size calculation.

🤔 mzumsande reviewed a pull request: "coins: make sure PoolAllocator uses the correct alignment"
(https://github.com/bitcoin/bitcoin/pull/28913#pullrequestreview-1738687795)
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)

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