💬 instagibbs commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1455418545)
Even better, I'll add a test that they're the same
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1455418545)
Even better, I'll add a test that they're the same
💬 instagibbs commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1455423851)
These are shorthand labels from https://delvingbitcoin.org/t/post-clustermempool-package-rbf-per-chunk-processing/190 which helped me think through preciesely how it maps onto the concepts laid out.
I'd like to keep track of these ideas somehow, maybe by defining the labels?
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1455423851)
These are shorthand labels from https://delvingbitcoin.org/t/post-clustermempool-package-rbf-per-chunk-processing/190 which helped me think through preciesely how it maps onto the concepts laid out.
I'd like to keep track of these ideas somehow, maybe by defining the labels?
💬 instagibbs commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1455426826)
can you be more explicit in suggestion? The errors are being returned?
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1455426826)
can you be more explicit in suggestion? The errors are being returned?
💬 instagibbs commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1455433932)
this is testing that subtraction works correctly, not that an individual chunk can have negative size
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1455433932)
this is testing that subtraction works correctly, not that an individual chunk can have negative size
📝 stickies-v opened a pull request: "refactor: remove CTxMemPool::queryHashes()"
(https://github.com/bitcoin/bitcoin/pull/29260)
`CTxMemPool::queryHashes()` is only used in `MempoolToJSON()`, where it can just as easily be replaced with the more general `CTxMemPool::entryAll()`. No behaviour change, just cleans up the code.
(https://github.com/bitcoin/bitcoin/pull/29260)
`CTxMemPool::queryHashes()` is only used in `MempoolToJSON()`, where it can just as easily be replaced with the more general `CTxMemPool::entryAll()`. No behaviour change, just cleans up the code.
🤔 furszy reviewed a pull request: "p2p: adaptive connections services flags"
(https://github.com/bitcoin/bitcoin/pull/28170#pullrequestreview-1827261571)
> If I understand it correctly, the logic deciding whether to try and accept outbound connections, using `HasAllDesirableServiceFlags()`, relies on `IsInitialBlockDownload()` / `DEFAULT_MAX_TIP_AGE` (24h) on master, but is changed to rely on `NODE_NETWORK_LIMITED_MIN_BLOCKS` (288 blocks, ~48h) in this PR.
>
> I think this means that the safety buffer of 144 blocks proposed in [BIP159](https://github.com/bitcoin/bips/blob/master/bip-0159.mediawiki) would have been removed. I'm not sure about th
...
(https://github.com/bitcoin/bitcoin/pull/28170#pullrequestreview-1827261571)
> If I understand it correctly, the logic deciding whether to try and accept outbound connections, using `HasAllDesirableServiceFlags()`, relies on `IsInitialBlockDownload()` / `DEFAULT_MAX_TIP_AGE` (24h) on master, but is changed to rely on `NODE_NETWORK_LIMITED_MIN_BLOCKS` (288 blocks, ~48h) in this PR.
>
> I think this means that the safety buffer of 144 blocks proposed in [BIP159](https://github.com/bitcoin/bips/blob/master/bip-0159.mediawiki) would have been removed. I'm not sure about th
...
💬 ismaelsadeeq commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1455527026)
I meant what is being returned by `err_string` and `CompareFeerateDiagram` was not used. why were you unable to compute mining score, same for `CompareFeerateDiagram` why is the fee rate insufficient.
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1455527026)
I meant what is being returned by `err_string` and `CompareFeerateDiagram` was not used. why were you unable to compute mining score, same for `CompareFeerateDiagram` why is the fee rate insufficient.
💬 instagibbs commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1455534078)
also, `Subtrack` isn't a word...
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1455534078)
also, `Subtrack` isn't a word...
💬 ismaelsadeeq commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1455539143)
Yeah, maybe to be consistent define the labels and do the same in the `OLD` `chunk` computation loop above.
Though the explicit comments in the `OLD` `chunk` computation loop is more easier to parse for me :).
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1455539143)
Yeah, maybe to be consistent define the labels and do the same in the `OLD` `chunk` computation loop above.
Though the explicit comments in the `OLD` `chunk` computation loop is more easier to parse for me :).
💬 Sjors commented on pull request "Support self hosted Github workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29259#discussion_r1455543617)
Hours and hours of headache :-)
Because I run four of those as different users, all builds would silently fail for the other users because they didn't have write permission on `/tmp/env`.
(https://github.com/bitcoin/bitcoin/pull/29259#discussion_r1455543617)
Hours and hours of headache :-)
Because I run four of those as different users, all builds would silently fail for the other users because they didn't have write permission on `/tmp/env`.
💬 ismaelsadeeq commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1455544265)
Yes, My question was... whats the rationale for `FeeFrac` to be generally have negative size ?
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1455544265)
Yes, My question was... whats the rationale for `FeeFrac` to be generally have negative size ?
💬 Sjors commented on pull request "Support self hosted Github workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29259#issuecomment-1895813220)
> A Cirrus account is free for public open source projects, as well as persistent workers connected to it.
Oh that's nice.
> GitHub "free" minutes should also be unlimited for public open source, no?
Ah, that could explain why my minutes aren't decreasing in Github settings despite having done a few CI runs. If that's documented somewhere then I'll drop a3b5b87a6a366579d6aa286956462dfadc6a13d9.
---
>> Related: I wrote a [gist](https://gist.github.com/Sjors/cd55370fa67aa095df90c433
...
(https://github.com/bitcoin/bitcoin/pull/29259#issuecomment-1895813220)
> A Cirrus account is free for public open source projects, as well as persistent workers connected to it.
Oh that's nice.
> GitHub "free" minutes should also be unlimited for public open source, no?
Ah, that could explain why my minutes aren't decreasing in Github settings despite having done a few CI runs. If that's documented somewhere then I'll drop a3b5b87a6a366579d6aa286956462dfadc6a13d9.
---
>> Related: I wrote a [gist](https://gist.github.com/Sjors/cd55370fa67aa095df90c433
...
💬 maflcko commented on pull request "Support self hosted Github workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29259#discussion_r1455566352)
Ah. May be better to use `$CONTAINER_NAME` then?
Could be a separate pull request, because this is a bug fix?
(https://github.com/bitcoin/bitcoin/pull/29259#discussion_r1455566352)
Ah. May be better to use `$CONTAINER_NAME` then?
Could be a separate pull request, because this is a bug fix?
💬 maflcko commented on pull request "Support self hosted Github workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29259#issuecomment-1895822246)
> But I've occasionally managed to trip up one or two CI instances on e.g. memory leaks. Sometimes I can reproduce them locally with a configure change, but other configurations are trickier. For those cases I can further improve the gist to only run a subset of the jobs.
In that case it seems easier to just run the "one or two" CI tasks on your machine that would become the "self-hosted runner", instead of going through the extra step to set up GitHub CI and the self-hosted runner with GHA?
(https://github.com/bitcoin/bitcoin/pull/29259#issuecomment-1895822246)
> But I've occasionally managed to trip up one or two CI instances on e.g. memory leaks. Sometimes I can reproduce them locally with a configure change, but other configurations are trickier. For those cases I can further improve the gist to only run a subset of the jobs.
In that case it seems easier to just run the "one or two" CI tasks on your machine that would become the "self-hosted runner", instead of going through the extra step to set up GitHub CI and the self-hosted runner with GHA?
💬 maflcko commented on pull request "Support self hosted Github workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29259#issuecomment-1895830004)
> > A Cirrus account is free for public open source projects, as well as persistent workers connected to it.
>
> Oh that's nice.
So it should be possible to achieve the same without touching any code here, by moving your GHA self-hosted runners to Cirrus.
Regardless, given that GHA minutes are "free", feel free to move the Cirrus self-hosted runners over to GHA-hosted runners. Not sure if this is trivially possible for all tasks, but if it is, it would make it easier to run the CI on fo
...
(https://github.com/bitcoin/bitcoin/pull/29259#issuecomment-1895830004)
> > A Cirrus account is free for public open source projects, as well as persistent workers connected to it.
>
> Oh that's nice.
So it should be possible to achieve the same without touching any code here, by moving your GHA self-hosted runners to Cirrus.
Regardless, given that GHA minutes are "free", feel free to move the Cirrus self-hosted runners over to GHA-hosted runners. Not sure if this is trivially possible for all tasks, but if it is, it would make it easier to run the CI on fo
...
💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1455588103)
yeah, leftover from the previous implementation. Cleaning them. Thanks!
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1455588103)
yeah, leftover from the previous implementation. Cleaning them. Thanks!
💬 Sjors commented on pull request "Support self hosted Github workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29259#issuecomment-1895839707)
It looks like Github's "test each commit" makes an assumption that doesn't hold for forks: https://github.com/Sjors/bitcoin/actions/runs/7556559882/job/20573828392?pr=28
(https://github.com/bitcoin/bitcoin/pull/29259#issuecomment-1895839707)
It looks like Github's "test each commit" makes an assumption that doesn't hold for forks: https://github.com/Sjors/bitcoin/actions/runs/7556559882/job/20573828392?pr=28
💬 Sjors commented on pull request "Support self hosted Github workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29259#issuecomment-1895847621)
> So it should be possible to achieve the same without touching any code here, by moving your GHA self-hosted runners to Cirrus.
I haven't looked into how self-hosted runners work with Cirrus. Adding another SAAS company into the loop seems suboptimal. And if Cirrus also reads all it configuration out of a yaml file in the repo, would it actually reduce the amount of code changes in the main repo?
(https://github.com/bitcoin/bitcoin/pull/29259#issuecomment-1895847621)
> So it should be possible to achieve the same without touching any code here, by moving your GHA self-hosted runners to Cirrus.
I haven't looked into how self-hosted runners work with Cirrus. Adding another SAAS company into the loop seems suboptimal. And if Cirrus also reads all it configuration out of a yaml file in the repo, would it actually reduce the amount of code changes in the main repo?
💬 maflcko commented on pull request "Support self hosted Github workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29259#issuecomment-1895848089)
> It looks like Github's "test each commit" makes an assumption that doesn't hold for forks: https://github.com/Sjors/bitcoin/actions/runs/7556559882/job/20573828392?pr=28
Yes, see https://github.com/Sjors/bitcoin/actions/runs/7556559882/job/20573828392?pr=28#step:4:42
(https://github.com/bitcoin/bitcoin/pull/29259#issuecomment-1895848089)
> It looks like Github's "test each commit" makes an assumption that doesn't hold for forks: https://github.com/Sjors/bitcoin/actions/runs/7556559882/job/20573828392?pr=28
Yes, see https://github.com/Sjors/bitcoin/actions/runs/7556559882/job/20573828392?pr=28#step:4:42
💬 maflcko commented on pull request "Support self hosted Github workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29259#issuecomment-1895850175)
> And if Cirrus also reads all it configuration out of a yaml file in the repo, would it actually reduce the amount of code changes in the main repo?
The workers currently use Cirrus self-hosted ones, so no changes are needed. This is documented in the cirrus yaml
(https://github.com/bitcoin/bitcoin/pull/29259#issuecomment-1895850175)
> And if Cirrus also reads all it configuration out of a yaml file in the repo, would it actually reduce the amount of code changes in the main repo?
The workers currently use Cirrus self-hosted ones, so no changes are needed. This is documented in the cirrus yaml