Bitcoin Core Github
42 subscribers
126K links
Download Telegram
👍 hodlinator approved a pull request: "init: Handle dropped UPnP support more gracefully"
(https://github.com/bitcoin/bitcoin/pull/31916#pullrequestreview-2645624370)
cr-ACK 44041ae0eca9d2034b7c2bdef24724809cc35e90

Most significant aspect is that it migrates users/nodes who previously enabled UPnP in the GUI to now use the new NATPMP/PCP support. Without this those nodes would probably have little chance of accepting inbound connections (depending on their LAN setup), until the user realizes their node is behaving differently (if ever).

#### Changes since [previous review](https://github.com/bitcoin/bitcoin/pull/31916#pullrequestreview-2641238999)

-
...
💬 ryanofsky commented on issue "[rfc] build: Reject unclean configure?":
(https://github.com/bitcoin/bitcoin/issues/31942#issuecomment-2685980592)
> cc [@ryanofsky](https://github.com/ryanofsky) I believe this is an alternative way to ease your concern from [#31161 (review)](https://github.com/bitcoin/bitcoin/pull/31161#pullrequestreview-2644640989), assuming that 31161 triggers a re-configure.

Thanks, this is interesting and I would agree that if we had a mechanism that would make it an error for the configure step to run after any build step had run, that would make the concern I had in #31161 moot. However, I think that change would al
...
🤔 ryanofsky reviewed a pull request: "doc: Update translation generation instructions"
(https://github.com/bitcoin/bitcoin/pull/31930#pullrequestreview-2645641106)
Code review ACK 75d5d235a6b5eb6b960be0c5e6e181460a1ac5e6. Looks good as a temporary fix and I think after #31741 we should be able to drop the extra argument.
🤔 l0rinc requested changes to a pull request: "doc: update fuzz instructions when on macOS"
(https://github.com/bitcoin/bitcoin/pull/31954#pullrequestreview-2645628866)
Concept ACK

Finally fuzzing is working with these changes (tested on M4 Max with Clang 19.1.7), but we're still getting warnings - and a I left a few more suggestions
💬 l0rinc commented on pull request "doc: update fuzz instructions when on macOS":
(https://github.com/bitcoin/bitcoin/pull/31954#discussion_r1972246194)
we can likely do this in a single command:
```suggestion
$ brew install llvm lld
```


We also likely need to remove the command from https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md?plain=1#L150 now
💬 l0rinc commented on pull request "doc: update fuzz instructions when on macOS":
(https://github.com/bitcoin/bitcoin/pull/31954#discussion_r1972242558)
Can you add a tracking issue here to know when we can safely remove it
💬 l0rinc commented on pull request "doc: update fuzz instructions when on macOS":
(https://github.com/bitcoin/bitcoin/pull/31954#discussion_r1972247440)
This seems to need an update - though I'd just remove the command from the comment:
```suggestion
Full configuration steps:
```
💬 l0rinc commented on pull request "doc: update fuzz instructions when on macOS":
(https://github.com/bitcoin/bitcoin/pull/31954#discussion_r1972250186)
I'm getting:
> clang++: warning: argument unused during compilation: '-fuse-ld=lld' [-Wunused-command-line-argument]
💬 m3dwards commented on pull request "doc: update fuzz instructions when on macOS":
(https://github.com/bitcoin/bitcoin/pull/31954#discussion_r1972452806)
Taken
💬 m3dwards commented on pull request "doc: update fuzz instructions when on macOS":
(https://github.com/bitcoin/bitcoin/pull/31954#discussion_r1972453011)
Took something similar
💬 m3dwards commented on pull request "doc: update fuzz instructions when on macOS":
(https://github.com/bitcoin/bitcoin/pull/31954#discussion_r1972453273)
Resolved warnings
💬 m3dwards commented on pull request "doc: update fuzz instructions when on macOS":
(https://github.com/bitcoin/bitcoin/pull/31954#discussion_r1972457053)
Not sure I feel this is big enough to add to the issue list and I would be happy for the docs to be updated when someone notices again that it's not needed any more, especially as this workaround should continue to work in the future even if strictly not needed any more.
💬 m3dwards commented on pull request "doc: update fuzz instructions when on macOS":
(https://github.com/bitcoin/bitcoin/pull/31954#discussion_r1972458293)
But obviously if you feel differently, go ahead and make the issue.
💬 m3dwards commented on pull request "doc: update fuzz instructions when on macOS":
(https://github.com/bitcoin/bitcoin/pull/31954#issuecomment-2686267479)
@Prabhat1308 and @l0rinc, I've updated the instructions to achieve the same goal but without the warnings.
💬 ayanwaar commented on issue "SignatureCreator should supply auxiliary data argument for additional bip340 signature security":
(https://github.com/bitcoin/bitcoin/issues/31883#issuecomment-2686280021)
https://github.com/bitcoin/bitcoin/issues/31883
💬 ayanwaar commented on something "":
(https://github.com/bitcoin/bitcoin/commit/db36a92c02b83f2e6477a5a55fc061319f7cc6a3#commitcomment-153053812)
db36a92c02b83f2e6477a5a55fc061319f7cc6a3
💬 ayanwaar commented on something "":
(https://github.com/bitcoin/bitcoin/commit/db36a92c02b83f2e6477a5a55fc061319f7cc6a3#r153053834)
db36a92c02b83f2e6477a5a55fc061319f7cc6a3
💬 ayanwaar commented on something "":
(https://github.com/bitcoin/bitcoin/commit/db36a92c02b83f2e6477a5a55fc061319f7cc6a3#r153053844)
db36a92c02b83f2e6477a5a55fc061319f7cc6a3
💬 l0rinc commented on pull request "doc: update fuzz instructions when on macOS":
(https://github.com/bitcoin/bitcoin/pull/31954#discussion_r1972485214)
I meant the `ld` issue - not critical