💬 MarcoFalke commented on pull request "ci: Prune dangling images on RESTART_CI_DOCKER_BEFORE_RUN":
(https://github.com/bitcoin/bitcoin/pull/27777#discussion_r1210414133)
I already switched all machines to `podman-docker`, because it is not possible to install `docker.io` and `podman-docker` at the same time. So `docker run` is actually just an alias for `podman run`.
(https://github.com/bitcoin/bitcoin/pull/27777#discussion_r1210414133)
I already switched all machines to `podman-docker`, because it is not possible to install `docker.io` and `podman-docker` at the same time. So `docker run` is actually just an alias for `podman run`.
💬 Sjors commented on pull request "wallet, bench: Move commonly used functions to their own file and fix a bug":
(https://github.com/bitcoin/bitcoin/pull/27666#issuecomment-1568589827)
utACK 7379a54ec416c8c0a029cc41835a23d42cb6d800
(https://github.com/bitcoin/bitcoin/pull/27666#issuecomment-1568589827)
utACK 7379a54ec416c8c0a029cc41835a23d42cb6d800
💬 MarcoFalke commented on pull request "ci: Prune dangling images on RESTART_CI_DOCKER_BEFORE_RUN":
(https://github.com/bitcoin/bitcoin/pull/27777#discussion_r1210416393)
Ok, could make sense in a follow-up to apply the label to both places and then unguard the `image prune` from `DANGER_RESTART_CI_DOCKER_BEFORE_RUN`?
(https://github.com/bitcoin/bitcoin/pull/27777#discussion_r1210416393)
Ok, could make sense in a follow-up to apply the label to both places and then unguard the `image prune` from `DANGER_RESTART_CI_DOCKER_BEFORE_RUN`?
💬 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.