Bitcoin Core Github
43 subscribers
122K links
Download Telegram
👍 ryanofsky approved a pull request: "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange, drop CRPCSignals & g_best_block"
(https://github.com/bitcoin/bitcoin/pull/30409#pullrequestreview-2303280869)
Code review ACK 76ec301c60e2530de7f58e2e4cea3d15c6a77cbc. Just simple rebase since last review
💬 Sjors commented on pull request "ci: honor ci bypass prefix in test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/30898#issuecomment-2349086465)
#19982 is an example of a PR which had an intermediate commit marked `[skip ci]`. Unfortunately I can't see how the CI handled that back then.
👍 ryanofsky approved a pull request: "Have createNewBlock() return a BlockTemplate interface"
(https://github.com/bitcoin/bitcoin/pull/30440#pullrequestreview-2303285049)
Code review ACK a93c171faa7b4426823466e972c8f24260918bbf. Since last review, just rebased with no changes or conflicts
🚀 fanquake merged a pull request: "build: add `standard branch-protection` to hardening flags for aarch64-linux"
(https://github.com/bitcoin/bitcoin/pull/30433)
💬 maflcko commented on pull request "ci: honor ci bypass prefix in test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/30898#issuecomment-2349100068)
1 commit every 4 years, or even 1 commit every year that qualifies for this just doesn't seem worth it the overhead.

If you want to break `git bisect`, there should be a good reason. A reason well enough that reviewers run the appropriate `git rebase --interactive --exec` locally. If there is no good enough reason, then the breaking change probably shouldn't be done.
💬 Sjors commented on pull request "ci: honor ci bypass prefix in test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/30898#issuecomment-2349119854)
> A reason well enough that reviewers run the appropriate git `rebase --interactive --exec` locally.

That seems a fairly arbitrary metric for measuring how good a reason is.

I'm more sympathetic to the possibility that both the reviewers and maintainer overlook a `[skip ci]` commit message (especially if it's not on the first line), causing `git bisect` to break. And worse, potentially allowing to sneak in a non-refactoring change.
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1758992957)
fixed
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1758993050)
good idea, moved this subtest to `mempool_dust.py` as its own commit, switched it to testing dustrelay arg instead of standardness as I think that's more directly testing what we want.
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1758993149)
dropped a note
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1758993261)
added suggestion and a direct assertion that the main output amount is >330 the taproot dust amount
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1758993345)
dropped the reference to modified as it probably isn't clarifying
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1758993386)
Yes, if it's fixed this will fail, letting the author know to write a new case. Added a note that this isn't desired, just descriptive,
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1758993580)
would cause the nodes to fail to sync their mempools, no? I find it easiest when the restarts are done in sync anyways.
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1758993663)
added a sentence to each saying when they should be called
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1758993726)
taken
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1758993801)
clarified comment, let me know if that's more clear
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1758993882)
taken
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1758993968)
added a hedge word
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1758994056)
nope, changed
🤔 pablomartin4btc reviewed a pull request: "test: Introduce ensure helper"
(https://github.com/bitcoin/bitcoin/pull/30893#pullrequestreview-2303325543)
Concept ACK

nit:
> - I have also been going back and forth on naming.

What about `ensure_during`? (another alternatives: `ensure_within_duration`/ `ensure_over_duration`) It adds a notion of time, making it more specific to the context of waiting or checking a condition over a period and it's less generic and more intuitive.