💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1149659725)
Removed unimplemented function

💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1149445980)
Yeah, it felt safer to first update everything, then to start deleting things. I did try combining the loops and it did not break anything, though. Still deciding, maybe this could be a follow-up.

💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1149399579)
I am not sure I follow. For historical reasons, transactions are included in their own ancestor set and their own descendant set. So while we are iterating over descendants, we could be evaluating a transaction itself that has no ancestors left after they were picked into our “virtual block”. If the evaluated transaction doesn’t have ancestors left, and we get its size with ancestors, we are comparing its own size with its own size in this inequality. Therefore, same size must be permitted.

I
...

💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1149653633)
Yeah, agreed. Gonna remove it

💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1149782804)
I’ve renamed the variable and better explained what’s going on here.

💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1149731957)
I’ve removed `all_entries`. Thanks for catching that

💬 willcl-ark commented on pull request "init: Error if ignored bitcoin.conf file is found":
(https://github.com/bitcoin/bitcoin/pull/27302#discussion_r1149825850)
I also do not get the error with:

```fish
₿ ./src/bitcoind -datadir=/tmp/bitcoin_27302/ -conf=/tmp/bitcoin_27302/bitcoin.conf -rpcport=18555 -port=18556
```

But I do see the error (as described in the commit bullet point 2) with:

```fish
₿ ./src/bitcoind -conf=/tmp/bitcoin_27302/bitcoin.conf -rpcport=18555 -port=18556
```

In my opinion:

> Show an error on startup if a bitcoin datadir that is being used contains a
> `bitcoin.conf` file that is ignored. There are two cases wher
...

📝 ismaelsadeeq opened a pull request: "refactor: use address_to_scriptpubkey instead of RPC call"
(https://github.com/bitcoin/bitcoin/pull/27349)
PR #27269 enables the function address_to_scriptpubkey() to decode all address types and return their corresponding scriptpubkeys. As a result, there is no longer any need to call getaddressinfo or validateaddress RPCs in order to retrieve an address scriptpubkey, as explained in the comments on this pull request (see https://github.com/bitcoin/bitcoin/pull/27269#pullrequestreview-1353681933 and https://github.com/bitcoin/bitcoin/pull/27269#issuecomment-1481016118).

Instead of using RPC calls
...

💬 ismaelsadeeq commented on pull request "refactor: use address_to_scriptpubkey instead of RPC call":
(https://github.com/bitcoin/bitcoin/pull/27349#issuecomment-1485964435)
@josibake @theStack @willcl-ark this is PR https://github.com/bitcoin/bitcoin/pull/27269 follow up, I will appreciate your reviews.
Thank you

💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#issuecomment-1485975148)
> Left few more comments.
>
> Plus, while was checking the 1p1c test, couldn't resist myself and made it more friendly [furszy@5dc4b89](https://github.com/furszy/bitcoin-core/commit/5dc4b8957c7a65746666cb5f3e7bd6270b21f8c2). Feel free to take it if you like it.

Sorry, I was just going through the commits one by one and only rediscovered this suggestion this evening. It looks like there is some good stuff there. I’ll aim to incorporate it shortly, but it might not be tonight.

📝 theStack opened a pull request: "test: refactor: dedup mempool_package_limits.py subtests via decorator"
(https://github.com/bitcoin/bitcoin/pull/27350)
The subtests in the functional test mempool_package_limits.py all follow the same pattern:
1. first, check that the mempool is currently empty
2. create and submit certain single txs to the mempool, prepare a to-be-submitted package
3. check that submitting the package fails with a "package-mempool-limits" error on each tx result
4. after mining a block, check that re-submitting the package succeeds

Note that steps 1,3,4 are identical for each of the subtests and only step 2 varies, so th
...

👍 theStack approved a pull request: "net: #27257 follow-ups"
(https://github.com/bitcoin/bitcoin/pull/27324)
ACK cd0c8eeb0940790b6ba83786d1c9e362d4dc4829

📝 ismaelsadeeq converted_to_draft a pull request: "test: use address_to_scriptpubkey instead of RPC call"
(https://github.com/bitcoin/bitcoin/pull/27349)
PR #27269 enables the function address_to_scriptpubkey() to decode all address types and return their corresponding scriptpubkeys. As a result, there is no longer any need to call getaddressinfo or validateaddress RPCs in order to retrieve an address scriptpubkey, as explained in the comments on this pull request (see https://github.com/bitcoin/bitcoin/pull/27269#pullrequestreview-1353681933 and https://github.com/bitcoin/bitcoin/pull/27269#issuecomment-1481016118).

Instead of using RPC calls
...

👋 ismaelsadeeq's pull request is ready for review: "test: use address_to_scriptpubkey instead of RPC call"
(https://github.com/bitcoin/bitcoin/pull/27349)

📝 ismaelsadeeq converted_to_draft a pull request: "test: use address_to_scriptpubkey instead of RPC call"
(https://github.com/bitcoin/bitcoin/pull/27349)
PR #27269 enables the function address_to_scriptpubkey() to decode all address types and return their corresponding scriptpubkeys. As a result, there is no longer any need to call getaddressinfo or validateaddress RPCs in order to retrieve an address scriptpubkey, as explained in the comments on this pull request (see https://github.com/bitcoin/bitcoin/pull/27269#pullrequestreview-1353681933 and https://github.com/bitcoin/bitcoin/pull/27269#issuecomment-1481016118).

Instead of using RPC calls
...

📝 apoelstra opened a pull request: "wallet: add `seeds` argument to `importdescriptors`"
(https://github.com/bitcoin/bitcoin/pull/27351)
This PR introduces the ability to import BIP32 master seeds alongside xpubs. It currently expects seeds to be provided in [BIP 93/codex32](https://github.com/bitcoin/bips/blob/master/bip-0093.mediawiki) either as a single seed or as a list of shares which can be assembled via Shamir Secret Sharing.

It could be generalized to other seed formats, e.g. a hex-encoding or the old `sethdseeds` WIF format, easily enough by just attempting to parse the input in those formats and retrying until we fin
...

💬 apoelstra commented on pull request "wallet: add `seeds` argument to `importdescriptors`":
(https://github.com/bitcoin/bitcoin/pull/27351#issuecomment-1486057058)
cc @achow101 @sipa @roconnor-blockstream

💬 ryanofsky commented on pull request "init: Error if ignored bitcoin.conf file is found":
(https://github.com/bitcoin/bitcoin/pull/27302#issuecomment-1486082157)
@willcl-ark this PR is not trying to error when one datadir setting takes precedence over another one. As the PR description states it adds an error for cases where "a bitcoin datadir **that is being used** contains a bitcoin.conf file that is ignored" (emphasis added). It does not show an error in cases where a bitcoin datadir that **not** being used contains a bitcoin.conf file that is ignored, because the whole directory is ignored, so there is nothing special about one file in it.

But we
...

💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1149975805)
I see, changing a pointer to a reference does not inherently prevent null pointer dereferences at runtime. However, by changing a pointer to a reference and documenting that the reference is expected to be non-null, we are effectively enforcing a contract that the caller of the function must provide a valid (non-null) reference. Suggestion taken, thanks!

💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1149977087)
I agree with this suggestion, first the manipulation of null values, mainly its return, and also removing the extra call to `evhttp_request_get_uri()`.