💬 stickies-v commented on pull request "build: Enable -Wunreachable-code":
(https://github.com/bitcoin/bitcoin/pull/28999#issuecomment-1841076279)
> You'll have to use `clang`, because GCC does not have those warnings.

I'm using clang, and it also caught the walletdb issue you fixed. It also catches e.g. `error.reset()` dead code I added. Just not the Assert.

```cpp
// ...and construct a CTxMemPool from it
std::optional<bilingual_str> error{};
return CTxMemPool{mempool_opts, error};
error.reset(); // just to add unreachable code
Assert(!error);
```

```
% make
Making all in src
GEN obj/build.h

...

💬 maflcko commented on pull request "build: Enable -Wunreachable-code":
(https://github.com/bitcoin/bitcoin/pull/28999#issuecomment-1841113048)
> Just not the Assert.

I thought I tested this, but looks like you are right. Any code from macros won't trigger the warning.

⚠️ martinus opened an issue: "Test `policyestimator_tests/BlockPolicyEstimates` failure"
(https://github.com/bitcoin/bitcoin/issues/29000)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

I have a reproducible test case failure on my machine. I've bisected it to commit 714523918ba2b853fc69bee6b04a33ba0c828bf5. Since this commit, the test case `policyestimator_tests/BlockPolicyEstimates`, but only when I use my builds script:

```sh
chrt -i 0 make -j32 check
```

`chrt` runs the make command in idle priority (I use that so I can smoothly browse while testing). I have a
...

💬 aureleoules commented on pull request "[POC] C++20 `std::endian`":
(https://github.com/bitcoin/bitcoin/pull/28674#issuecomment-1841125275)
I've been experimenting with benchmark comparisons between pulls and master on my test coverage tool corecheck (https://corecheck.dev/bitcoin/bitcoin/pulls/28674).
It seems that this pull request is negatively impacting a bunch of benchmarks:

Note that the benchmarks are being run on ARM64 CPUs, but I tested locally on an x86 and the performance loss is roughly the same.

💬 maflcko commented on issue "Test `policyestimator_tests/BlockPolicyEstimates` failure":
(https://github.com/bitcoin/bitcoin/issues/29000#issuecomment-1841149630)
You can fix it by adding `SyncWithValidationInterfaceQueue` in line 113, no?

📝 instagibbs opened a pull request: "Ephemeral Anchors"
(https://github.com/bitcoin/bitcoin/pull/29001)
Depends on https://github.com/bitcoin/bitcoin/pull/28948 and https://github.com/bitcoin/bitcoin/pull/28984

Replaces https://github.com/bitcoin/bitcoin/pull/26403 to refresh the conversation.

BIP text here: https://github.com/instagibbs/bips/blob/ephemeral_anchor/bip-ephemeralanchors.mediawiki

TODO:
1) figure out what precisely to do in a reorg when ephemeral transactions are trying to enter the mempool(and write a test)

✅ instagibbs closed a pull request: "policy: Ephemeral anchors"
(https://github.com/bitcoin/bitcoin/pull/26403)

💬 sr-gi commented on pull request "test: Extends MEMPOOL msg functional test":
(https://github.com/bitcoin/bitcoin/pull/28485#discussion_r1415936588)
I don't think it is. Did a bunch of testing locally and it may be a remnant of the old approach taken for this test, which does not longer apply.

I've reversed some of the calls to make the diff as minimal as possible

💬 maflcko commented on pull request "test: Extends MEMPOOL msg functional test":
(https://github.com/bitcoin/bitcoin/pull/28485#issuecomment-1841170942)
lgtm ACK 59e86afbcdfb9dbf52a6580246233ee501c51475

💬 theStack commented on pull request "script/sign: avoid duplicated signature verification after signing (+introduce signing benchmarks)":
(https://github.com/bitcoin/bitcoin/pull/28923#issuecomment-1841192171)
> Does this reduce our safety against memory corruption or similar?

I don't think so, at least I don't see how verifying a created signature twice in a row has any benefit over doing it only once.

⚠️ maflcko opened an issue: "Intermittent test failure in p2p_v2_transport"
(https://github.com/bitcoin/bitcoin/issues/29002)
https://drahtbot.space/temp_scratch/p2p_v2_transport_129.tar.xz

```
test 2023-12-05T14:06:56.900000Z TestFramework.utils (ERROR): wait_until() failed. Predicate: ''''
self.wait_until(lambda: len(self.nodes[0].getpeerinfo()) == num_peers + 1)
'''
test 2023-12-05T14:06:56.902000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):

...

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/28765#issuecomment-1841206307)
re-ACK [3a062b2](https://github.com/bitcoin/bitcoin/pull/28765/commits/3a062b2bdc6dc787b967947872f55131522cd2ac) the diff is mainly moving the removal of TODOs between commits.

I've noticed that the co-authorship of 3a062b2bdc6dc787b967947872f55131522cd2ac was dropped, which may have been unintended.

Also, looks like this is failing CI, but it may be unrelated.

💬 martinus commented on issue "Test `policyestimator_tests/BlockPolicyEstimates` failure":
(https://github.com/bitcoin/bitcoin/issues/29000#issuecomment-1841221668)
Right above the end of the loop? Yes, this seems to do the trick, I've run it 10 times and it always worked

💬 mzumsande commented on issue "getrawtransaction xxxxxx.... 2 causes a segfault":
(https://github.com/bitcoin/bitcoin/issues/28986#issuecomment-1841237473)
I found the issue and could reproduce it, it only happens with a pruned node.
[This line](https://github.com/bitcoin/bitcoin/blob/6d5790956f45e3de5c6c4ee6fda21878b0d1287b/src/rpc/rawtransaction.cpp#L406) calls `IsBlockPruned(blockindex)`, and if the tx is in the mempool, `blockindex` is a nullptr. If also `m_have_pruned` is true, dereferencing `pblockindex->nStatus` will cause a crash. Will open a fix.

💬 maflcko commented on pull request "p2p: Fill reconciliation sets (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/28765#discussion_r1416006054)
Seems better to use a deterministic fast random context in tests, so that failures, if they happen, are deterministic?

💬 fanquake commented on pull request "[POC] C++20 `std::endian`":
(https://github.com/bitcoin/bitcoin/pull/28674#issuecomment-1841276845)
cc @theuni we might not always be getting bswaps

💬 techy2 commented on issue "getrawtransaction xxxxxx.... 2 causes a segfault":
(https://github.com/bitcoin/bitcoin/issues/28986#issuecomment-1841278626)
relevant parts of config
daemon=1
#txindex=1
server=1
#listen=1
shrinkdebuglog=1
prune=2000
mempoolfullrbf=1

👍 theStack approved a pull request: "test: Extends MEMPOOL msg functional test"
(https://github.com/bitcoin/bitcoin/pull/28485#pullrequestreview-1765724477)
re-ACK 59e86afbcdfb9dbf52a6580246233ee501c51475

💬 theuni commented on pull request "[POC] C++20 `std::endian`":
(https://github.com/bitcoin/bitcoin/pull/28674#issuecomment-1841329489)
@aureleoules Thanks for testing!

I tested this by comparing the x86_64 asm output of a simple test file and confirmed that it compiled down to bswaps as necessary. I didn't do the same for other arches. Perhaps others are missing the critical optimizations? :(

Could you say more about your compiler/flags? I would expect to see the nasty behavior you're seeing:
- On old compilers
- On non-bleeding-edge MSVC
- Without optimizations.

Would you mind pasting the flags use for compile? Wit
...

🤔 pablomartin4btc reviewed a pull request: "rpc: addpeeraddress tried return error on failure"
(https://github.com/bitcoin/bitcoin/pull/28998#pullrequestreview-1765800678)
Concept ACK