Bitcoin Core Github
44 subscribers
121K links
Download Telegram
πŸ’¬ 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.
πŸ’¬ fanquake commented on pull request "guix: build `bitcoin-qt` with static libxcb & utils":
(https://github.com/bitcoin/bitcoin/pull/33537#discussion_r2522793220)
Fixed.
πŸ’¬ stickies-v commented on pull request "kernel: add btck_block_tree_entry_equals":
(https://github.com/bitcoin/bitcoin/pull/33855#discussion_r2522796090)
_response to https://github.com/bitcoin/bitcoin/pull/33855#issuecomment-3526237850_

@alexanderwiederin opening this thread for easier focus. Are you suggesting we 1) implement `btck_block_tree_entry_equals` with a hash equality check, or 2) to not expose `btck_block_tree_entry_equals` at all and let the user define it however they want (such as using hash equality), or 3) something else?

If 1), I don't see the benefit? Pointer comparisons are faster, and are conceptually correct, and what
...
πŸ’¬ maflcko commented on pull request "ci: Lint follow-ups":
(https://github.com/bitcoin/bitcoin/pull/33776#issuecomment-3527196104)
> 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.

Agree, but I think we can use warning notices to bubble up more important ones.



> Without knowing exactly how this machine readable annotation is being consumed or experimenting more, I'm not aware of an alternative?

It can be used via the API, e.g. https://github.com/maflcko/DrahtBot/blob/6c91378116516ca5974d78a4e0c5c495b38acf95/webhook_feature
...