💬 jamesob commented on pull request "assumeutxo (2)":
(https://github.com/bitcoin/bitcoin/pull/27596#discussion_r1210425053)
@Sjors this is bash :). If he's on macOS, probably some kind of zsh?
(https://github.com/bitcoin/bitcoin/pull/27596#discussion_r1210425053)
@Sjors this is bash :). If he's on macOS, probably some kind of zsh?
💬 pinheadmz commented on pull request "net: Allow inbound whitebind connections to more aggressively evict peers when slots are full":
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1210427226)
That's a good point, `whitelist` is harder to attack than `whitebind` because the attacker would have to spoof their origin IP repeatedly to fill up your inbounds. If a user `whitebind`'s a port and an attacker figures out that port numnber, they can trivially evict all your other inbounds
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1210427226)
That's a good point, `whitelist` is harder to attack than `whitebind` because the attacker would have to spoof their origin IP repeatedly to fill up your inbounds. If a user `whitebind`'s a port and an attacker figures out that port numnber, they can trivially evict all your other inbounds
💬 theuni commented on pull request "kernel: Remove shutdown from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27711#issuecomment-1568618485)
> > It also provides an easy way to see when calls to shutdown are made without returning to the calling function.
>
> Can you explain this? Am I correct when guessing that any added EXIT* macro, and not the BUBBLE* macros are indicating a missing return?
@MarcoFalke Right, the `BUBBLE_UP` is kinda like a cast, it's not so interesting. The others introduce conditional returns to pass any errors back up the stack. So the problem cases are ones where we don't already have that behavior.
T
...
(https://github.com/bitcoin/bitcoin/pull/27711#issuecomment-1568618485)
> > It also provides an easy way to see when calls to shutdown are made without returning to the calling function.
>
> Can you explain this? Am I correct when guessing that any added EXIT* macro, and not the BUBBLE* macros are indicating a missing return?
@MarcoFalke Right, the `BUBBLE_UP` is kinda like a cast, it's not so interesting. The others introduce conditional returns to pass any errors back up the stack. So the problem cases are ones where we don't already have that behavior.
T
...
✅ dergoegge closed a pull request: "[PoC] Structure aware fuzzing with libprotobuf-mutator"
(https://github.com/bitcoin/bitcoin/pull/26975)
(https://github.com/bitcoin/bitcoin/pull/26975)
🚀 fanquake merged a pull request: "wallet, bench: Move commonly used functions to their own file and fix a bug"
(https://github.com/bitcoin/bitcoin/pull/27666)
(https://github.com/bitcoin/bitcoin/pull/27666)
💬 dergoegge commented on pull request "p2p: Prevent block index fingerprinting by sending additional getheaders messages":
(https://github.com/bitcoin/bitcoin/pull/24571#issuecomment-1568632278)
Closing for now, can be marked up for grabs.
Someone should look into implementing the alternative approach I have described here: https://github.com/bitcoin/bitcoin/pull/24571#issuecomment-1475286226.
(https://github.com/bitcoin/bitcoin/pull/24571#issuecomment-1568632278)
Closing for now, can be marked up for grabs.
Someone should look into implementing the alternative approach I have described here: https://github.com/bitcoin/bitcoin/pull/24571#issuecomment-1475286226.
✅ dergoegge closed a pull request: "p2p: Prevent block index fingerprinting by sending additional getheaders messages"
(https://github.com/bitcoin/bitcoin/pull/24571)
(https://github.com/bitcoin/bitcoin/pull/24571)
💬 achow101 commented on pull request "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`":
(https://github.com/bitcoin/bitcoin/pull/26261#issuecomment-1568642975)
ACK 5c832c3820253affc65c0ed490e26e5b0a4d5c9b
(https://github.com/bitcoin/bitcoin/pull/26261#issuecomment-1568642975)
ACK 5c832c3820253affc65c0ed490e26e5b0a4d5c9b
🚀 achow101 merged a pull request: "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`"
(https://github.com/bitcoin/bitcoin/pull/26261)
(https://github.com/bitcoin/bitcoin/pull/26261)
👋 achow101's pull request is ready for review: "wallet: cache IsMine scriptPubKeys to improve performance of descriptor wallets"
(https://github.com/bitcoin/bitcoin/pull/26008)
(https://github.com/bitcoin/bitcoin/pull/26008)
💬 MarcoFalke commented on pull request "kernel: Remove shutdown from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27711#issuecomment-1568675044)
> Edit: to be clear, the problem here is that FlushUndoFile may call AbortNode, but we don't react to that in WriteUndoDataForBlock.
Do we have to react there? `AbortNode` starts a shutdown, and the shutdown is later checked in ABC: https://github.com/bitcoin/bitcoin/blob/71300489af362c3fed4736de6bffab4d758b6a84/src/validation.cpp#L3193
Sure, better annotations are better, but then why not use the existing `Result` class over the `MaybeEarlyExit` type?
(https://github.com/bitcoin/bitcoin/pull/27711#issuecomment-1568675044)
> Edit: to be clear, the problem here is that FlushUndoFile may call AbortNode, but we don't react to that in WriteUndoDataForBlock.
Do we have to react there? `AbortNode` starts a shutdown, and the shutdown is later checked in ABC: https://github.com/bitcoin/bitcoin/blob/71300489af362c3fed4736de6bffab4d758b6a84/src/validation.cpp#L3193
Sure, better annotations are better, but then why not use the existing `Result` class over the `MaybeEarlyExit` type?
💬 MarcoFalke commented on pull request "assumeutxo (2)":
(https://github.com/bitcoin/bitcoin/pull/27596#discussion_r1210488998)
macOS ships an ancient version of bash
(https://github.com/bitcoin/bitcoin/pull/27596#discussion_r1210488998)
macOS ships an ancient version of bash
💬 pablomartin4btc commented on pull request "httpserver, rest: improving URI validation":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1210497550)
ok
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1210497550)
ok
⚠️ torkelrogstad opened an issue: "Unclear documentation about TX replacements in `gettransaction`"
(https://github.com/bitcoin/bitcoin/issues/27781)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
The two fields about TXID replacement in `gettransaction` currently says this:
```
"replaced_by_txid" : "hex", (string, optional) The txid if this tx was replaced.
"replaces_txid" : "hex", (string, optional) The txid if the tx replaces one.
```
From looking at the [code](https://github.com/bitcoin/bitcoin/blob/71300489af362c3fed4736de6bffab4d758b6a84/
...
(https://github.com/bitcoin/bitcoin/issues/27781)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
The two fields about TXID replacement in `gettransaction` currently says this:
```
"replaced_by_txid" : "hex", (string, optional) The txid if this tx was replaced.
"replaces_txid" : "hex", (string, optional) The txid if the tx replaces one.
```
From looking at the [code](https://github.com/bitcoin/bitcoin/blob/71300489af362c3fed4736de6bffab4d758b6a84/
...
💬 theuni commented on pull request "kernel: Remove shutdown from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27711#issuecomment-1568701692)
> > Edit: to be clear, the problem here is that FlushUndoFile may call AbortNode, but we don't react to that in WriteUndoDataForBlock.
>
> Do we have to react there? `AbortNode` starts a shutdown, and the shutdown is later checked in ABC:
>
> https://github.com/bitcoin/bitcoin/blob/71300489af362c3fed4736de6bffab4d758b6a84/src/validation.cpp#L3193
>
Yes, looks like I picked this example too hastily. You're correct that this one is caught higher up.
> Sure, better annotations are bet
...
(https://github.com/bitcoin/bitcoin/pull/27711#issuecomment-1568701692)
> > Edit: to be clear, the problem here is that FlushUndoFile may call AbortNode, but we don't react to that in WriteUndoDataForBlock.
>
> Do we have to react there? `AbortNode` starts a shutdown, and the shutdown is later checked in ABC:
>
> https://github.com/bitcoin/bitcoin/blob/71300489af362c3fed4736de6bffab4d758b6a84/src/validation.cpp#L3193
>
Yes, looks like I picked this example too hastily. You're correct that this one is caught higher up.
> Sure, better annotations are bet
...
💬 MarcoFalke commented on issue "Unclear documentation about TX replacements in `gettransaction`":
(https://github.com/bitcoin/bitcoin/issues/27781#issuecomment-1568703383)
Pull requests welcome :)
(https://github.com/bitcoin/bitcoin/issues/27781#issuecomment-1568703383)
Pull requests welcome :)
📝 torkelrogstad opened a pull request: "wallet: clarify replace fields in gettransaction"
(https://github.com/bitcoin/bitcoin/pull/27782)
Closes #27781.
(https://github.com/bitcoin/bitcoin/pull/27782)
Closes #27781.
💬 theuni commented on pull request "macOS: Bump minimum required runtime version and prepare for building with upstream LLVM":
(https://github.com/bitcoin/bitcoin/pull/27676#issuecomment-1568710074)
After discussing offline with @fanquake, it doesn't make sense to let a single test keep us from upgrading our entire darwin toolchain infrastructure. I've removed the lief check for now until we can bump to >= 0.13.0.
(https://github.com/bitcoin/bitcoin/pull/27676#issuecomment-1568710074)
After discussing offline with @fanquake, it doesn't make sense to let a single test keep us from upgrading our entire darwin toolchain infrastructure. I've removed the lief check for now until we can bump to >= 0.13.0.
💬 theuni commented on issue "Adding a minimally-patched Guix repo to the org":
(https://github.com/bitcoin/bitcoin/issues/25098#issuecomment-1568719209)
Reopening for https://github.com/bitcoin/bitcoin/pull/27676.
One of those commits introduces a new guix fork in order to [patch the LLVM build](https://github.com/theuni/guix/commits/fix-clang-includes). In this case the typical overrides don't work, and forking llvm as @dongcarl suggested here seems to be the most straightforward solution. I intend to upstream the changes eventually, but even if accepted that means a (potentially lengthy) delay and a time machine bump.
If there are no ob
...
(https://github.com/bitcoin/bitcoin/issues/25098#issuecomment-1568719209)
Reopening for https://github.com/bitcoin/bitcoin/pull/27676.
One of those commits introduces a new guix fork in order to [patch the LLVM build](https://github.com/theuni/guix/commits/fix-clang-includes). In this case the typical overrides don't work, and forking llvm as @dongcarl suggested here seems to be the most straightforward solution. I intend to upstream the changes eventually, but even if accepted that means a (potentially lengthy) delay and a time machine bump.
If there are no ob
...
⚠️ theuni reopened an issue: "Adding a minimally-patched Guix repo to the org"
(https://github.com/bitcoin/bitcoin/issues/25098)
As of e6a94d44469f90f4dc88a07a5a8587730811c705, we use Guix's upstream git repo on https://git.savannah.gnu.org/git/guix.git for our `guix time-machine`-powered pinning.
However, it does seem that we occasionally will encounter problems with Guix's upstream repo which are not easily fixed using the Guix package transformation flags.
In particular:
1. A test of `gnutls` failed because of a chronological error whereby a certificate used in a test expired. (#21203)
- We waited until G
...
(https://github.com/bitcoin/bitcoin/issues/25098)
As of e6a94d44469f90f4dc88a07a5a8587730811c705, we use Guix's upstream git repo on https://git.savannah.gnu.org/git/guix.git for our `guix time-machine`-powered pinning.
However, it does seem that we occasionally will encounter problems with Guix's upstream repo which are not easily fixed using the Guix package transformation flags.
In particular:
1. A test of `gnutls` failed because of a chronological error whereby a certificate used in a test expired. (#21203)
- We waited until G
...