Bitcoin Core Github
44 subscribers
121K links
Download Telegram
πŸ“ maflcko opened a pull request: "refactor: Avoid -W*-whitespace in git archive"
(https://github.com/bitcoin/bitcoin/pull/33869)
Otherwise, compilation with GCC-15+ will warn about it:

```
src/clientversion.cpp:33:79: error: trailing whitespace [-Werror=trailing-whitespace=]
33 | //! git will put "#define GIT_COMMIT_ID ..." on the next line inside archives.
```

Follow-up to https://github.com/bitcoin/bitcoin/pull/32482#issuecomment-3522280482

Can be tested via `git archive --output=/tmp/a.tar HEAD`
πŸ’¬ maflcko commented on pull request "miner: drop dummy extraNonce in coinbase scriptSig for templates requested via IPC":
(https://github.com/bitcoin/bitcoin/pull/32420#discussion_r2522490088)

/**\n+ * Whether to include and OP_0 as a dummy extraNonce in the template's coinbase\n+ */\n -> "and" -> "an" [Correct article: "an OP_0" (singular noun) makes the sentence grammatical and clear.]
πŸ’¬ maflcko commented on pull request "refactor: Improve assumeutxo state representation":
(https://github.com/bitcoin/bitcoin/pull/30214#discussion_r2435927466)
llm nits:


assumetxo -> assumeutxo [spelling mistake in comments; inconsistent with the rest of the code/documentation]
assumetxo -> assumeutxo [same misspelling repeated elsewhere in the added enum comments]
contains database -> contains a database [missing article makes the sentence ungrammatical / harder to parse]
πŸ’¬ maflcko commented on pull request "tests: Add witness commitment if we have a witness transaction in `FullBlockTest.update_block()`":
(https://github.com/bitcoin/bitcoin/pull/31823#discussion_r2522534594)

doesnt -> doesn’t [missing apostrophe in contraction β€œdoesn't”]
βœ… maflcko closed a pull request: "tests: Add witness commitment if we have a witness transaction in `FullBlockTest.update_block()`"
(https://github.com/bitcoin/bitcoin/pull/31823)
πŸ“ maflcko reopened a pull request: "tests: Add witness commitment if we have a witness transaction in `FullBlockTest.update_block()`"
(https://github.com/bitcoin/bitcoin/pull/31823)
This is useful for test cases where we want to test logic invalid blocks that contain witness transactions. If we don't add the witness commitment as per [BIP141](https://github.com/bitcoin/bips/blob/master/bip-0141.mediawiki#user-content-Commitment_structure), blocks will be rejected with the error [`Block mutated`](https://github.com/bitcoin/bitcoin/blob/fb0ada982a73687520c43b8fde480fa5d456f3e1/src/validation.cpp#L4180).

This change was needed in https://github.com/ajtowns/bitcoin/pull/13 w
...
πŸ’¬ fanquake commented on pull request "refactor: Avoid -W*-whitespace in git archive":
(https://github.com/bitcoin/bitcoin/pull/33869#issuecomment-3526740723)
Should we modify one job to use the archive? We should probably be catching, and minimising any differences, like this, between building from source, and the tarball.
πŸ’¬ maflcko commented on pull request "refactor: Avoid -W*-whitespace in git archive":
(https://github.com/bitcoin/bitcoin/pull/33869#issuecomment-3526775978)
> Should we modify one job to use the archive?

Possibly, but I don't think any job is running with gcc-15, so the benefits here would be limited. Though, I can modify my nightly matrix to use archives explicitly, instead of by accident of git not being present.



> We should probably be catching, and minimising any differences, like this, between building from source, and the tarball.

I want to keep the changes here minimal. Reworking this is already tracked in https://github.com/bitc
...
πŸ’¬ m3dwards commented on pull request "ci: Lint follow-ups":
(https://github.com/bitcoin/bitcoin/pull/33776#discussion_r2522600040)
Missing import time?
πŸ’¬ m3dwards commented on pull request "ci: Lint follow-ups":
(https://github.com/bitcoin/bitcoin/pull/33776#discussion_r2522613366)
Should be `build_cmd`
πŸ’¬ vasild commented on pull request "net_processing: rename RelayTransaction to better describe what it does":
(https://github.com/bitcoin/bitcoin/pull/33565#issuecomment-3526831112)
`9989853447...78595ae0e7`: take suggestions
βœ… fanquake closed an issue: "fees: leftover references to `policy/fees.cpp`"
(https://github.com/bitcoin/bitcoin/issues/33863)
πŸš€ fanquake merged a pull request: "scripted-diff: fix leftover references to `policy/fees.h`"
(https://github.com/bitcoin/bitcoin/pull/33864)
πŸ’¬ vasild commented on pull request "net_processing: rename RelayTransaction to better describe what it does":
(https://github.com/bitcoin/bitcoin/pull/33565#discussion_r2522652086)
This slipped through, thanks for the reminder. Done now and also took the changes into https://github.com/bitcoin/bitcoin/pull/29415
πŸ’¬ vasild commented on pull request "net_processing: rename RelayTransaction to better describe what it does":
(https://github.com/bitcoin/bitcoin/pull/33565#discussion_r2522653126)
Done
πŸ’¬ vasild commented on pull request "net_processing: rename RelayTransaction to better describe what it does":
(https://github.com/bitcoin/bitcoin/pull/33565#discussion_r2522654868)
Done
πŸ’¬ m3dwards commented on pull request "ci: Lint follow-ups":
(https://github.com/bitcoin/bitcoin/pull/33776#issuecomment-3526866633)
The GHA behaviour of removing annotations from previous runs is very annoying! I experimented with having an upstream job that printed the annotation and having all the other jobs depend on it but again when you rerun a failed downstream job you lose the annotation from the run upstream job.

It's a shame that in this PR we have to have the annotation printed many times, one per job, as I think it might drown out other notices. Without knowing exactly how this machine readable annotation is be
...
πŸ’¬ TheCharlatan commented on pull request "refactor: Let CCoinsViewCache::BatchWrite return void":
(https://github.com/bitcoin/bitcoin/pull/33866#issuecomment-3526892334)
Updated 6d1818888b1e56dec6c57fadb3a99bfacbab742f -> d1823ebdd32d106668a3a8085da31e274d5c0ac1 ([batch_write_void_0](https://github.com/TheCharlatan/bitcoin/tree/batch_write_void_0) -> [batch_write_void_1](https://github.com/TheCharlatan/bitcoin/tree/batch_write_void_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/batch_write_void_0..batch_write_void_1))

* Addressed @andrewtoth's [comment](https://github.com/bitcoin/bitcoin/pull/33866#discussion_r2520746299), use `NextAndMaybeEras
...
πŸ’¬ vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3526942592)
`875795ef0f...7547f915bc`: take suggestions from https://github.com/bitcoin/bitcoin/pull/33565
πŸ’¬ fanquake commented on pull request "refactor: Avoid -W*-whitespace in git archive":
(https://github.com/bitcoin/bitcoin/pull/33869#issuecomment-3527012975)
> Possibly, but I don't think any job is running with gcc-15, so the benefits here would be limited.

I'm just thinking generally, not about this single warning, or gcc-15. Happy for it to best changed in a followup, or in another related change.