Bitcoin Core Github
44 subscribers
121K links
Download Telegram
πŸ’¬ fjahr commented on pull request "Embedded ASMap [3/3]: Build binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2532010167)
Probably not a bad idea in principle but the nature of the test changes with your suggested code, so I would like to manage the PR scope a bit and keep this for a follow-up.
πŸ’¬ fjahr commented on pull request "Embedded ASMap [3/3]: Build binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2532054758)
Addressed in #33878

> (Also removes data from file-global scope).

Not a problem because of the anonymous namespace I think.
πŸ’¬ fjahr commented on pull request "Embedded ASMap [3/3]: Build binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2532014755)
Addressed in https://github.com/bitcoin/bitcoin/pull/33878
πŸ’¬ fjahr commented on pull request "Embedded ASMap [3/3]: Build binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2532014382)
I have been working on improved documentation. Please check the new commit in https://github.com/bitcoin/bitcoin/pull/33878 to see if that is fixing this completely.
πŸ’¬ fjahr commented on pull request "Embedded ASMap [3/3]: Build binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2532224663)
Addressed in https://github.com/bitcoin/bitcoin/pull/33878
πŸ’¬ fjahr commented on pull request "Embedded ASMap [3/3]: Build binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2532022415)
Done in #33878
πŸ’¬ fjahr commented on pull request "Embedded ASMap [3/3]: Build binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#issuecomment-3539393301)
All the feedback that applied to https://github.com/bitcoin/bitcoin/pull/33878 has now been addressed there. I am marking these comments as resolved and if there are further notes on these please move the conversation over there. Thanks!
πŸ’¬ fjahr commented on pull request "refactor, docs: Embedded ASMap [2/3]: Refactor asmap internals and add documentation":
(https://github.com/bitcoin/bitcoin/pull/33878#issuecomment-3539395607)
All the feedback from #28792 that applied to the commits here has been addressed here now.
πŸ’¬ fjahr commented on pull request "refactor, docs: Embedded ASMap [2/3]: Refactor asmap internals and add documentation":
(https://github.com/bitcoin/bitcoin/pull/33878#discussion_r2532262781)
Thank you for the quick feedback! Added a small paragraph on the bit-packed format in the introduction comment at the top.
πŸ’¬ fjahr commented on issue "interface_ipc functional test failing in CI":
(https://github.com/bitcoin/bitcoin/issues/33884#issuecomment-3539397600)
I have opened #33880 to address this.
πŸ’¬ starius commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-3539405058)
> Are you still working on this?

Yes. I'm addressing all the feedback. I'll publish new version soon.
βœ… pinheadmz closed an issue: "Improve Bitcoin’s resilience to large-scale power grid failures and Carrington-type solar storms"
(https://github.com/bitcoin/bitcoin/issues/33885)
πŸ’¬ pinheadmz commented on issue "Improve Bitcoin’s resilience to large-scale power grid failures and Carrington-type solar storms":
(https://github.com/bitcoin/bitcoin/issues/33885#issuecomment-3539414510)
This should be posted on the [bitcoin-dev mailing list](https://groups.google.com/g/bitcoindev), the [Delving Bitcoin forum](https://delvingbitcoin.org/) or some other platform where broad, protocol-level concepts are discussed. Conceptual questions and most usage questions can be posted on [Stack Exchange](https://bitcoin.stackexchange.com/). The Bitcoin Core issue tracker is reserved for discussion about this specific software project only, its implementation and usage.
πŸ’¬ fjahr commented on pull request "build: Embedded ASMap [3/3]: Build binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2532296702)
One might say that "NO" is _literally_ the inversion of "ON" 😜

Changed in the latest push :)
πŸ’¬ fjahr commented on pull request "build: Embedded ASMap [3/3]: Build binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2532296866)
Done
πŸ’¬ fjahr commented on pull request "build: Embedded ASMap [3/3]: Build binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2532299269)
Missed the bit about the commit message initially, now also addressed in #33878
πŸ’¬ fjahr commented on pull request "build: Embedded ASMap [3/3]: Build binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#issuecomment-3539450021)
This is now in sync with #33026 and https://github.com/bitcoin/bitcoin/pull/33878 again so technically ready for review but of course the other PRs should be checked out first.
πŸ‘‹ fjahr's pull request is ready for review: "build: Embedded ASMap [3/3]: Build binary dump header file"
(https://github.com/bitcoin/bitcoin/pull/28792)
πŸ’¬ theStack commented on pull request "test: Fix reorg patterns in tests to use proper fork-based approach":
(https://github.com/bitcoin/bitcoin/pull/32587#issuecomment-3539520136)
Concept ACK

Looks like half of the `create_empty_fork` method moving slipped into the second commit?
πŸ’¬ ajtowns commented on pull request "mining: add getCoinbase()":
(https://github.com/bitcoin/bitcoin/pull/33819#issuecomment-3539952304)
> Deprecate `getCoinbaseTx()` in favor of a new method that provides a struct with everything clients need to construct a coinbase. This is safer than providing a raw dummy coinbase that clients then have to manipulate.

That seems backwards to me:

1. Constructing a coinbase requires getting various consensus requirements correct, and new requirements may be added in future, such as the nlocktime requirement proposed in bip54. Why not keep all this logic in the node / template generator?

...