💬 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

💬 maflcko commented on pull request "[POC] C++20 `std::endian`":
(https://github.com/bitcoin/bitcoin/pull/28674#issuecomment-1841373951)
I'd presume it is https://github.com/corecheck/coverage-worker/blob/7d4767493be390399b54ea3cd8cafc2f068c19e2/entrypoint.sh#L58 (default flags `-O2` on Ubuntu Jammy)

📝 mzumsande opened a pull request: "rpc: fix getrawtransaction segfault"
(https://github.com/bitcoin/bitcoin/pull/29003)
The crash, reported in #28986, happens when calling `getrawtransaction` for any mempool transaction with `verbosity=2`, while pruning, because the rpc calls `IsBlockPruned(const CBlockIndex* pblockindex)`, which dereferences `pblockindex` without a check.

For ease of backporting this PR fixes it just locally in `rpc/rawtransaction.cpp` by moving the check for`!blockindex` up so that the `IsBlockPruned()` will not be called with a `nullptr`. We might also want to change `IsBlockPruned()` so it
...

💬 mzumsande commented on issue "getrawtransaction xxxxxx.... 2 causes a segfault":
(https://github.com/bitcoin/bitcoin/issues/28986#issuecomment-1841379445)
See #29003 for a fix - thanks a lot for reporting, I'm really surprised that no one else has run into this before!

💬 achow101 commented on pull request "doc: explain what the wallet password does":
(https://github.com/bitcoin/bitcoin/pull/28974#discussion_r1416123781)
We should avoid portraying the seed as something that is recommended for restoring a wallet. The seed is not easily exported and doing so is not a recommended method of backing up a wallet. Also not all encrypted wallets will have a seed, and not all private keys in a wallet are necessarily derived from the same seed, if derived at all.

💬 aureleoules commented on pull request "[POC] C++20 `std::endian`":
(https://github.com/bitcoin/bitcoin/pull/28674#issuecomment-1841400148)
> I'd presume it is [corecheck/coverage-worker@7d47674/entrypoint.sh#L58](https://github.com/corecheck/coverage-worker/blob/7d4767493be390399b54ea3cd8cafc2f068c19e2/entrypoint.sh#L58) (default flags -O2 on Ubuntu Jammy)

Yes that is correct. Also the compiler installed is `g++-11`.

💬 kilrau commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1841406432)
Is there another source for up-to-date full-rbf pool adoption?

💬 maflcko commented on pull request "rpc: fix getrawtransaction segfault":
(https://github.com/bitcoin/bitcoin/pull/29003#issuecomment-1841418135)
lgtm ACK 494a926d05df44b60b3bc1145ad2a64acf96f61b

A test wouldn't hurt, I'd say.

💬 maflcko commented on pull request "rpc: fix getrawtransaction segfault":
(https://github.com/bitcoin/bitcoin/pull/29003#issuecomment-1841426498)
I guess this was introduced in https://github.com/bitcoin/bitcoin/commit/f86697163e5cdbc3bc4a65cfb7dbaa3d9eb602a9

💬 nogamike76 commented on pull request "rpc: fix getrawtransaction segfault":
(https://github.com/bitcoin/bitcoin/pull/29003#issuecomment-1841429818)
No it wouldn’t. What needs to be done

May God guide us and may we all, including myself find happiness, love and
forgiveness. Be kind to someone, help someone with something that would
help them. Make sure you tel your family you love them. May the kings be
protected and may our King, Jesus Christ, always be the divine and glory,
in honor of our father, the almighty one, God.
Spirit Leader
Michael Noga jr.
Descendant of King David
Nogah, son of David
T8<3
$€£¥


On Tue, Dec 5, 20
...

💬 maflcko commented on pull request "rpc: fix getrawtransaction segfault":
(https://github.com/bitcoin/bitcoin/pull/29003#discussion_r1416148547)
unrelated: I don't understand the silent fallback to verbose=1, when verbose=2 is not available. Either this should be documented, or an error should be thrown (same below).

Though, this is unrelated.

💬 maflcko commented on pull request "rpc: fix getrawtransaction segfault":
(https://github.com/bitcoin/bitcoin/pull/29003#issuecomment-1841435462)
> We might also want to change `IsBlockPruned()` so it doesn't crash when called with a `nullptr`, but I didn't do that here.

I think we should avoid the use of pointers when they are assumed to not be null. In C++, references exit. However, this can be done in a follow-up.

💬 aureleoules commented on pull request "[POC] C++20 `std::endian`":
(https://github.com/bitcoin/bitcoin/pull/28674#issuecomment-1841445584)
Here is the output of the configure script of the corecheck job if this helps:

```
1701686294337,checking for pkg-config... /usr/bin/pkg-config
1701686294338,checking pkg-config is at least version 0.9.0... yes
1701686294378,checking build system type... aarch64-unknown-linux-gnu
1701686294379,checking host system type... aarch64-unknown-linux-gnu
1701686294384,checking for a BSD-compatible install... /usr/bin/install -c
1701686294387,checking whether build environment is sane... yes
1
...