💬 stickies-v commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1600159941)
In 7e475b9648bbee04f5825b922ba0399373eaa5a9:

I think having the `if/else` path in this commit is helpful, since it distinguishes between when we cast the txid and when we don't.

nit: I think this already works pretty well, but some bits (e.g. "guess what the wtxid is") are not super clear imo. I've summarized my understanding into the below, feel free to use what you like.

```
// Never query by txid: it is possible that an existing transaction in the orphanage has the

...

💬 stickies-v commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1600235908)
c++20-nit:
```suggestion
return m_orphans.contains(wtxid);
```

💬 stickies-v commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1600337905)
nit: I first interpreted that as that an input had been removed from the transaction, perhaps better to use "parent" as to stay consistent with the language in the next steps?
```suggestion
# 1. Fake orphan is received first, the parent is not yet broadcast.
```

💬 stickies-v commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1600252935)
In 8923edfc1f12ebc6a074651c084ba7d249074799:

nit: I feel like documentation on why `TxOrphanage` indexes by wtxid/allows multiple versions of the same transaction should be documented in `TxOrphanage` instead of here.

💬 stickies-v commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1600356156)
nit: would `tx_middle_bad_wit` be more consistent naming here?

💬 itornaza commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#issuecomment-2110867498)
trACK 0fb17bf61a40b73a2b81a18e70b3de180c917f22

💬 mzumsande commented on pull request "blockstorage: Separate reindexing from saving new blocks":
(https://github.com/bitcoin/bitcoin/pull/29975#discussion_r1600509299)
done

💬 mzumsande commented on pull request "blockstorage: Separate reindexing from saving new blocks":
(https://github.com/bitcoin/bitcoin/pull/29975#discussion_r1600510097)
Agree. Not only for error-checking, we'd also expect users to do something with the `FlatFilePos` that `FindNextBlockPos` now returns.

💬 mzumsande commented on pull request "blockstorage: Separate reindexing from saving new blocks":
(https://github.com/bitcoin/bitcoin/pull/29975#discussion_r1600512963)
Sorry, missing the context here, is this suggesting a change?

💬 mzumsande commented on pull request "blockstorage: Separate reindexing from saving new blocks":
(https://github.com/bitcoin/bitcoin/pull/29975#discussion_r1600513102)
fixed

💬 mzumsande commented on pull request "blockstorage: Separate reindexing from saving new blocks":
(https://github.com/bitcoin/bitcoin/pull/29975#discussion_r1600513951)
Expanded the comment with the `fKnown` behavior.

💬 mzumsande commented on pull request "blockstorage: Separate reindexing from saving new blocks":
(https://github.com/bitcoin/bitcoin/pull/29975#discussion_r1600514427)
done as suggested.

💬 mzumsande commented on pull request "blockstorage: Separate reindexing from saving new blocks":
(https://github.com/bitcoin/bitcoin/pull/29975#discussion_r1600514845)
moved to the doc commit.

💬 mzumsande commented on pull request "blockstorage: Separate reindexing from saving new blocks":
(https://github.com/bitcoin/bitcoin/pull/29975#issuecomment-2110943056)
[9cf475f ](https://github.com/bitcoin/bitcoin/commit/9cf475ffffb869cd55c2b2f3be84d7c90b199521)to [e41667b](https://github.com/bitcoin/bitcoin/commit/e41667b720372dae8438ea86e9819027e62b54e0):
Addressed review feedback.

💬 pinheadmz commented on pull request "include verbose "debug-message" field in testmempoolaccept response":
(https://github.com/bitcoin/bitcoin/pull/28121#issuecomment-2110960190)
force-push to fix merge conflict which also looked like a git mistake on my part in the first place 😬

👍 hebasto approved a pull request: "util: avoid using thread_local variable that has a destructor"
(https://github.com/bitcoin/bitcoin/pull/30095#pullrequestreview-2056250408)
ACK c5f9afd946efb4eb6b27b2d0316f2f787a9608c7, tested on FreeBSD 14.0.

`./src/bitcoind -logthreadnames` works as expected.

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/28765#discussion_r1600564410)
I'm guessing "a peer for some reason does not request reconciliations from us for a long while", hence why it references (1)

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/28765#discussion_r1600586129)
This seems to be consistent with how a deterministic randomizer is seeded in many other places in the codebase. What is your rationale for making it different here?

⚠️ edilmedeiros opened an issue: "contrib/signet/miner: grind will fail for high difficulty chain"
(https://github.com/bitcoin/bitcoin/issues/30102)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Mining will fail with a `Could not satisfy difficulty target`.

```bash
❯ ~/2-development/bitcoin/bitcoin-core/contrib/signet/miner --cli "/Users/jose.edil/2-development/bitcoin/bitcoin-core/src/bitcoin-cli -datadir=/Users/jose.edil/2-development/bitcoin/signet-mining-experiments/signet-fix-hashing" generate --address tb1qylfujt900rjxzfxjj7sktpu7dpm2n9j60ch7jt --grind-cmd "/Users/jose.e
...

💬 TheCharlatan commented on pull request "serialization: Support for multiple parameters":
(https://github.com/bitcoin/bitcoin/pull/28929#discussion_r1600617406)
Are you going to be using this method for other purposes besides tests and as an internal getter?