🤔 tdb3 reviewed a pull request: "rpc: allow writing UTXO set to a named pipe, introduce dump_to_sqlite.sh script"
(https://github.com/bitcoin/bitcoin/pull/31560#pullrequestreview-2522042088)
Approach ACK

Great feature!

Did some manual santiy testing on mainnet:
- Used `dumptxoutset` to create a dump (with a node synced to block 876,186), `utxo_to_sqlite.py` to covert to a sqlite file, and opened/parsed in python. Conversion seemed successful, the correct number of coins were present in the table
- Used `dump_to_sqlite.sh` to do the same with fifo (but with a node synced to block 200,000), then open/parsed in python. Conversion seemed successful, the correct number of coins
...

💬 tdb3 commented on pull request "rpc: allow writing UTXO set to a named pipe, introduce dump_to_sqlite.sh script":
(https://github.com/bitcoin/bitcoin/pull/31560#discussion_r1896861260)
Rather than have this wait indefinitely, might be better to specify a `timeout` (e.g. `CONVERSION_TIMEOUT = 60`, `p.wait(timeout=CONVERSION_TIMEOUT)`). This could allow earlier failure detection (i.e. instead of relying on the longer CI timeout).

💬 tdb3 commented on pull request "rpc: allow writing UTXO set to a named pipe, introduce dump_to_sqlite.sh script":
(https://github.com/bitcoin/bitcoin/pull/31560#discussion_r1896865020)
`2024-present`

💬 tdb3 commented on pull request "rpc: allow writing UTXO set to a named pipe, introduce dump_to_sqlite.sh script":
(https://github.com/bitcoin/bitcoin/pull/31560#discussion_r1896864227)
`2024-present`

💬 tdb3 commented on pull request "rpc: allow writing UTXO set to a named pipe, introduce dump_to_sqlite.sh script":
(https://github.com/bitcoin/bitcoin/pull/31560#discussion_r1896811520)
nit: To prevent maintaining the same info (table structure) in multiple files, maybe we can create a link here to the opening comment in `utxo_to_sqlite.py` (describing the table)?

💬 tdb3 commented on pull request "rpc: allow writing UTXO set to a named pipe, introduce dump_to_sqlite.sh script":
(https://github.com/bitcoin/bitcoin/pull/31560#discussion_r1896861901)
Might be less churn to have commit d9a8a137b64f586422455318a9e757b1967a4f73 introduce this function instead of refactoring in this commit.

🤔 sipa reviewed a pull request: "descriptor: remove unreachable verification for `pkh`"
(https://github.com/bitcoin/bitcoin/pull/31555#pullrequestreview-2522148711)
crACK 366ae00b779acd59a61719422f0597acb17fb3e0

👍 ismaelsadeeq approved a pull request: "rpc: Extend scope of validation mutex in generateblock"
(https://github.com/bitcoin/bitcoin/pull/31563#pullrequestreview-2522159692)
Code review and tested ACK fa62c8b1f04a5386ffa171aeff713d55bd874cbe

I was able to recreate the failure by calling `generateblock` concurrently from multiple threads using https://gist.github.com/ismaelsadeeq/a5e3edb3fbf2d7f3c6698c1610622d22

```terminal
2024-12-24T17:10:44Z Saw new header hash=34f195658f52e6cf693a2cb3dbec8d421ef7523168ea18ba5d4fb89f4d631596 height=1301
2024-12-24T17:10:44Z [validation] NewPoWValidBlock: block hash=34f195658f52e6cf693a2cb3dbec8d421ef7523168ea18ba5d4fb8
...

🤔 ismaelsadeeq reviewed a pull request: "cluster mempool: introduce TxGraph"
(https://github.com/bitcoin/bitcoin/pull/31363#pullrequestreview-2522187404)
Concept ACK


> GetChunkFeerate (get the mining score of a transaction)


From reading the description and skimming through the code, IIUC `TxGraph` encapsulates knowledge about fees and sizes but lacks knowledge about prioritization. This implies that the fee it uses is the effective fee, not the modified fee that includes prioritization.

How are we accounting for the real mining score of a `Ref` without that prioritization knowledge? The current mining algorithm uses the modified anc
...

💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#issuecomment-2561368066)
@ismaelsadeeq TxGraph (and DepGraph, and FeeFrac) just treat "fee" and "size" as numbers whose ratio is to be maximized. These classes don't care what meaning they correspond to.

In practice, I expect that the mempool code will provide the modified fee as "fee", and vsize or weight as "size".

⚠️ Andresmore96 opened an issue: "Bitcoin"
(https://github.com/bitcoin/bitcoin/issues/31565)

✅ fanquake closed an issue: "Bitcoin"
(https://github.com/bitcoin/bitcoin/issues/31565)

:lock: fanquake locked an issue: "Bitcoin"
(https://github.com/bitcoin/bitcoin/issues/31565)

💬 ismaelsadeeq commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#issuecomment-2561403310)
Thank you for clarifying. Reading 'lacks knowledge about prioritization' led me to infer that.
But I see now you are talking about `mapDeltas`

📝 lucasbalieiro opened a pull request: "test: add unit tests for SigningResultString function"
(https://github.com/bitcoin/bitcoin/pull/31566)
This PR introduces new unit tests for the `SigningResultString` function located in the file [src/common/signmessage.cpp](https://github.com/bitcoin/bitcoin/blob/master/src/common/signmessage.cpp).

#### **Motivation**:
Prior to this PR, the `SigningResultString` function had no test coverage. This update adds the necessary test cases to ensure that the function behaves as expected.

#### **Changes**:
- Added unit tests to cover all the cases in the switch statement within the `SigningResu
...

💬 furszy commented on pull request "descriptors: inference process, do not return unparsable multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31404#discussion_r1896957545)
> ```
> assert_equal('multisig', rpc_result['type'])
> ```
> Nit: This assertion can also be added. I modified the test case to have 21 keys, and the `type` changed to `nonstandard`, which is correct. `p2sh` and `desc` values are present in that output.
>
> Do you think it's worth adding the test case with 21 pubkeys as well to cover the `nonstandard` scenario?

Hmm, I find the `type` return field misleading. Based on the code, it relates to the output script type, which in this case can
...

📝 lucasbalieiro converted_to_draft a pull request: "test: add unit tests for SigningResultString function"
(https://github.com/bitcoin/bitcoin/pull/31566)
This PR introduces new unit tests for the `SigningResultString` function located in the file [src/common/signmessage.cpp](https://github.com/bitcoin/bitcoin/blob/master/src/common/signmessage.cpp).

#### **Motivation**:
Prior to this PR, the `SigningResultString` function had no test coverage. This update adds the necessary test cases to ensure that the function behaves as expected.

#### **Changes**:
- Added unit tests to cover all the cases in the switch statement within the `SigningResu
...

✅ lucasbalieiro closed a pull request: "[WIP] test: add unit tests for SigningResultString function"
(https://github.com/bitcoin/bitcoin/pull/31566)

📝 lucasbalieiro opened a pull request: "test: add unit tests for SigningResultString function"
(https://github.com/bitcoin/bitcoin/pull/31567)
This PR introduces new unit tests for the `SigningResultString` function located in the file [src/common/signmessage.cpp](https://github.com/bitcoin/bitcoin/blob/master/src/common/signmessage.cpp).

#### **Motivation**:
Prior to this PR, the `SigningResultString` function had no test coverage. This update adds the necessary test cases to ensure that the function behaves as expected.

#### **Changes**:
- Added unit tests to cover all the cases in the switch statement within the `SigningResu
...

💬 besoeasy commented on issue "HardFork (SLH-DSA)":
(https://github.com/bitcoin/bitcoin/issues/31095#issuecomment-2561485150)
@elapsedluck good post, but better to create a BIP