Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 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
💬 luke-jr commented on pull request "test: fix MIN macro redefinition":
(https://github.com/bitcoin/bitcoin/pull/31419#issuecomment-2561489156)
Should we do this in `contrib/tracing/log_raw_p2p_msgs.py` too?
🤔 luke-jr reviewed a pull request: "util: detect and warn when using exFAT on MacOS"
(https://github.com/bitcoin/bitcoin/pull/31453#pullrequestreview-2522253623)
Would be ideal if we had a way to reproduce the bug, and can test if the user is actually affected, in case Apple fixes it. But this is better than nothing for now
💬 luke-jr commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r1897019800)
For GUI, maybe we should check this while they still have the opportunity to change it...