Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 ismaelsadeeq commented on issue "Fee Estimation via Fee rate Forecasters tracking issue":
(https://github.com/bitcoin/bitcoin/issues/30392#issuecomment-2405172852)
> Both using sat/vB units.

This is a good idea.

> Beginning with a setfeerate RPC in https://github.com/bitcoin/bitcoin/pull/20391.

I learned about this issue after @murchandamus's review comment https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1526664705. I think we should just unify `settxfee` and your `setfeerate` into one RPC?

> Was considering picking these up again. Presumably orthogonal, apart from coordinating on the RPC naming.

Happy to review when you do.
...
💬 fanquake commented on pull request "build: Rename `PACKAGE_*` variables to `CLIENT_*`":
(https://github.com/bitcoin/bitcoin/pull/31042#discussion_r1795498723)
I think if we end up doingthat, we should extend this to renaming other source variables that exist in the PACKAGE_ namespace. This PR as-is is already changing things such that naming becomes more inconsistent in that regard.
💬 instagibbs commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1795560402)
this is definitely problematic, but I can't seem to figure out a way to hit it. In prod this would "just" cause it to not call `m_txrequest.ForgetTxHash(ptx->GetWitnessHash());` for it, but would have to be in a 1p1c package setting.

To retain current behavior, I think it can just be expanded to
```suggestion
// DIFFERENT_WITNESS has "valid" status, but we still want to stop requesting it
if (!Assume(state.IsInvalid() ||
tx_result.m_result_type == MempoolAcceptResult::Res
...
📝 danielabrozzoni opened a pull request: "rest: Support transaction broadcast in REST interface"
(https://github.com/bitcoin/bitcoin/pull/31065)
This adds a REST endpoint to allow broadcasting a transaction:
`POST /rest/broadcast.hex`

The transaction hex must be passed in the body of the request;
on success, the txid of the broadcasted transaction will be returned.

Fixes #31017
💬 ryanofsky commented on pull request "Introduce Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30200#issuecomment-2405280136)
> Is it ok for other external users of `processNewBlock` to skip over the pre-checks and optimizations that are normally done in `submitblock`?

I assumed it was ok, but now that `interfaces::BlockTemplate::submitSolution` in added in 525e9dcba0b8c6744bcd3725864f39786afc8ed5 (to deal with the concern in https://github.com/bitcoin/bitcoin/pull/30440#issuecomment-2229233817) maybe the `interfaces::Mining::processNewBlock` method could be dropped and RPC code can go back to calling `ProcessNewBlo
...
💬 glozow commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1795517895)
tiny typo unusued
🤔 glozow reviewed a pull request: "cluster mempool: extend DepGraph functionality"
(https://github.com/bitcoin/bitcoin/pull/30857#pullrequestreview-2360504869)
ACK 0b3ec8c59b2b6d598531cd4e688eb276e597c825, reviewed range-diff from aab53ddcd8fcbc3c0be0da9383f8e06abe5badda and `clusterlin_depgraph_sim`
💬 glozow commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1795548751)
Nice, no more naming clash for `Cluster` in my mind
🚀 glozow merged a pull request: "cluster mempool: extend DepGraph functionality"
(https://github.com/bitcoin/bitcoin/pull/30857)
⚠️ maflcko opened an issue: " scriptpubkeyman fuzz target TopUp is slow (3/N)"
(https://github.com/bitcoin/bitcoin/issues/31066)
Follow-up to https://github.com/bitcoin/bitcoin/issues/30476 and https://github.com/bitcoin/bitcoin/issues/30541:

Found by OSS-Fuzz: https://issues.oss-fuzz.com/issues/369374541

Probably a duplicate of, or similarity with https://github.com/bitcoin/bitcoin/issues/30498

File: [clusterfuzz-testcase-scriptpubkeyman-6582005385461760.not.txt](https://github.com/user-attachments/files/17330258/clusterfuzz-testcase-scriptpubkeyman-6582005385461760.not.txt)



```
$ FUZZ=scriptpubkeyman pe
...
💬 vasild commented on issue "build: RFC Coverage build type":
(https://github.com/bitcoin/bitcoin/issues/31047#issuecomment-2405471187)
> I liked the summary stats per folder produced by the current approach, but if others like other stuff better, I don't mind.

Maybe I misunderstand, but somehow the above implies that we have to commit to one and give up on the other method. But one can create gcc or clang coverages regardless of what is integrated into the cmake build system. That integration would be just a convenience and we could have no integration, or just gcc (the current `master`), or just clang or both gcc and clang.
...
👋 danielabrozzoni's pull request is ready for review: "rest: Support transaction broadcast in REST interface"
(https://github.com/bitcoin/bitcoin/pull/31065)
👍 vasild approved a pull request: "netinfo: add peer services column and outbound-only peers list"
(https://github.com/bitcoin/bitcoin/pull/30930#pullrequestreview-2360849799)
ACK 683b558a020f1632b0a3cbdaa165adbd1423281c
💬 vasild commented on pull request "netinfo: add peer services column and outbound-only peers list":
(https://github.com/bitcoin/bitcoin/pull/30930#discussion_r1795748339)
Before these numbers somehow designated increasing verbosity and more columns. Whereas now the newly added "5" does not add more columns, but _removes rows_ compared to "4".

I am not sure how to improve that. It is a different dimension, maybe use a different option "outonly". Then one could have either one of the 0-4 modes in "outonly" mode or in full mode (in + out).

(non-blocker, just something to consider)
💬 maflcko commented on issue "build: RFC Coverage build type":
(https://github.com/bitcoin/bitcoin/issues/31047#issuecomment-2405561186)
Yeah. I guess I don't really mind either way. As long as there is at least one documented, tested and working solution.
📝 maflcko opened a pull request: "test: Print CompletedProcess object on error"
(https://github.com/bitcoin/bitcoin/pull/31067)
It would be good to know the output on `Error parsing command output`. Otherwise test failures are meaningless: https://github.com/bitcoin/bitcoin/issues/30792#issuecomment-2325911157

Fix it by just printing the full `CompletedProcess` object.

Also, use the modern `subprocess.run` to simplify the code.
💬 jonatack commented on pull request "netinfo: add peer services column and outbound-only peers list":
(https://github.com/bitcoin/bitcoin/pull/30930#discussion_r1795801949)
I see, but think it's simpler and more convenient for the user (i.e. me) to type 5 rather than 4, or vice versa.

I did maintain the "high details levels = maximum" behavior preferred by @laanwj in https://github.com/bitcoin/bitcoin/pull/21192 for values above 5.
💬 jonatack commented on pull request "netinfo: add peer services column and outbound-only peers list":
(https://github.com/bitcoin/bitcoin/pull/30930#discussion_r1795804652)
Also, I find myself only using level 4 or 5. Perhaps other reviewers would prefer the option to apply, as you suggest, to all the other levels. Happy to do that (here or in a follow-up) if people prefer.
💬 sipa commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#issuecomment-2405669097)
Rebased after the merge of #30857.
⚠️ ArmchairCryptologist opened an issue: "Consider making 27.x Long-Term Support (LTS)"
(https://github.com/bitcoin/bitcoin/issues/31068)
### Please describe the feature you'd like to see added.

As mentioned in the patch notes for 28.x, since this release drops support for glibc older than 2.31, it will not run on several widely used Linux distros, including RHEL 8 and the RHELatives that base off of this, such as AlmaLinux 8 and Rocky Linux 8.

In case anyone is unaware, RHEL 8 and its derivatives are still supported, and will in fact not be EOL until 2029. While I don't have exact install numbers, I would not be surprised if
...