Bitcoin Core Github
44 subscribers
120K links
Download Telegram
⚠️ 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
💬 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.