π¬ hebasto commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#discussion_r2367227845)
@davidgumberg
> I think this approach is no less invasive then what is done currently using an external script...
I disagree. I've checked c44fb4facfd3a5db8aa1bd2c7f5506a1477b0d2b. The `env:` sections in the CI logs are indeed more bloated compared to the master branch. I'd prefer to avoid this.
(https://github.com/bitcoin/bitcoin/pull/32513#discussion_r2367227845)
@davidgumberg
> I think this approach is no less invasive then what is done currently using an external script...
I disagree. I've checked c44fb4facfd3a5db8aa1bd2c7f5506a1477b0d2b. The `env:` sections in the CI logs are indeed more bloated compared to the master branch. I'd prefer to avoid this.
π¬ hebasto commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#issuecomment-3317735731)
For the second commit, you might consider using a YAML anchor:
```diff
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -211,7 +211,8 @@ jobs:
steps:
- *CHECKOUT
- - name: Set up VS Developer Prompt
+ - &SET_UP_VS
+ name: Set up VS Developer Prompt
shell: pwsh -Command "$PSVersionTable; $PSNativeCommandUseErrorActionPreference = $true; $ErrorActionPreference = 'Stop'; & '{0}'"
run: |
$vswherePath = "${env:Pr
...
(https://github.com/bitcoin/bitcoin/pull/32513#issuecomment-3317735731)
For the second commit, you might consider using a YAML anchor:
```diff
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -211,7 +211,8 @@ jobs:
steps:
- *CHECKOUT
- - name: Set up VS Developer Prompt
+ - &SET_UP_VS
+ name: Set up VS Developer Prompt
shell: pwsh -Command "$PSVersionTable; $PSNativeCommandUseErrorActionPreference = $true; $ErrorActionPreference = 'Stop'; & '{0}'"
run: |
$vswherePath = "${env:Pr
...
π¬ hebasto commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#issuecomment-3317763545)
> While in this area of the code, this PR also runs some additional manifest checks on the windows binaries.
> ... there are still two places where `shell: pwsh` is used without your ` -Command "$PSVersionTable; $PSNativeCommandUseErrorActionPreference = $true; $ErrorActionPreference = 'Stop'; & '{0}'"` suffix...
While I do agree with the concerns above, it might be better to keep this PR focused on resolving https://github.com/bitcoin/bitcoin/issues/32508.
(https://github.com/bitcoin/bitcoin/pull/32513#issuecomment-3317763545)
> While in this area of the code, this PR also runs some additional manifest checks on the windows binaries.
> ... there are still two places where `shell: pwsh` is used without your ` -Command "$PSVersionTable; $PSNativeCommandUseErrorActionPreference = $true; $ErrorActionPreference = 'Stop'; & '{0}'"` suffix...
While I do agree with the concerns above, it might be better to keep this PR focused on resolving https://github.com/bitcoin/bitcoin/issues/32508.
π¬ sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2367465700)
Brought it back in 4ead575b252becbfa18ad28a7bfa7fbf3de7129d
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2367465700)
Brought it back in 4ead575b252becbfa18ad28a7bfa7fbf3de7129d
π¬ sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2367502150)
Squashed this commit.
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2367502150)
Squashed this commit.
π¬ vasild commented on pull request "system: improve handling around GetTotalRAM()":
(https://github.com/bitcoin/bitcoin/pull/33435#issuecomment-3318101491)
`8fcf71ca00...56791b5829`: take suggestions
(https://github.com/bitcoin/bitcoin/pull/33435#issuecomment-3318101491)
`8fcf71ca00...56791b5829`: take suggestions
π¬ vasild commented on pull request "system: improve handling around GetTotalRAM()":
(https://github.com/bitcoin/bitcoin/pull/33435#discussion_r2367509262)
Done.
(https://github.com/bitcoin/bitcoin/pull/33435#discussion_r2367509262)
Done.
π¬ vasild commented on pull request "system: improve handling around GetTotalRAM()":
(https://github.com/bitcoin/bitcoin/pull/33435#discussion_r2367509839)
Done.
(https://github.com/bitcoin/bitcoin/pull/33435#discussion_r2367509839)
Done.
π¬ sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2367523246)
Did the latter, thanks.
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2367523246)
Did the latter, thanks.
π hebasto approved a pull request: "system: improve handling around GetTotalRAM()"
(https://github.com/bitcoin/bitcoin/pull/33435#pullrequestreview-3251759768)
ACK 56791b582958e905e5ba5cbf172a8ea7dad1a8b0.
(https://github.com/bitcoin/bitcoin/pull/33435#pullrequestreview-3251759768)
ACK 56791b582958e905e5ba5cbf172a8ea7dad1a8b0.
π¬ sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2367557960)
Done in 7cebfec8a6197a260e8c21f4637fad5d60163cfa
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2367557960)
Done in 7cebfec8a6197a260e8c21f4637fad5d60163cfa
π¬ sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2367560636)
Fixed.
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2367560636)
Fixed.
π¬ sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2367572401)
Should be better now
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2367572401)
Should be better now
π bitschmidty opened a pull request: "datacarrier: Undeprecate configuration option"
(https://github.com/bitcoin/bitcoin/pull/33453)
Removes the deprecation for the `datacarrier` and `datacarriersize` options by reverting commit 0b4048c73385166144d0b3e76beb9a2ac4cc1eca from https://github.com/bitcoin/bitcoin/pull/32406
**Many current Bitcoin Core users want to continue using this option**
This statement is based on public postings from many Bitcoin Core users and not a formal survey. AJ Townsβ observation from [#32406](https://github.com/bitcoin/bitcoin/pull/32406/commits/0b4048c73385166144d0b3e76beb9a2ac4cc1eca#r20840248
...
(https://github.com/bitcoin/bitcoin/pull/33453)
Removes the deprecation for the `datacarrier` and `datacarriersize` options by reverting commit 0b4048c73385166144d0b3e76beb9a2ac4cc1eca from https://github.com/bitcoin/bitcoin/pull/32406
**Many current Bitcoin Core users want to continue using this option**
This statement is based on public postings from many Bitcoin Core users and not a formal survey. AJ Townsβ observation from [#32406](https://github.com/bitcoin/bitcoin/pull/32406/commits/0b4048c73385166144d0b3e76beb9a2ac4cc1eca#r20840248
...
π¬ sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2367581993)
Fixed now.
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2367581993)
Fixed now.
π¬ bitschmidty commented on pull request "datacarrier: Undeprecate configuration option":
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3318166185)
I open this PR with the goal of having the deprecation removed for the upcoming v30 release, in order to minimize any additional confusion around this option.
I ask that any Bitcoin Core users that want to indicate support or opposition to this PR without providing technical feedback use the π or π reactions rather than an ACK/NACK in hopes of keeping this PR easy and expeditious to review.
_Disclaimer: I am the executive director of Brink, an organization that funds some Bitcoin Core de
...
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3318166185)
I open this PR with the goal of having the deprecation removed for the upcoming v30 release, in order to minimize any additional confusion around this option.
I ask that any Bitcoin Core users that want to indicate support or opposition to this PR without providing technical feedback use the π or π reactions rather than an ACK/NACK in hopes of keeping this PR easy and expeditious to review.
_Disclaimer: I am the executive director of Brink, an organization that funds some Bitcoin Core de
...
π¬ sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2367587173)
Done.
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2367587173)
Done.
β
sipa closed a pull request: "Exponentially-decaying tx inventory queue"
(https://github.com/bitcoin/bitcoin/pull/33449)
(https://github.com/bitcoin/bitcoin/pull/33449)
π¬ sipa commented on pull request "Exponentially-decaying tx inventory queue":
(https://github.com/bitcoin/bitcoin/pull/33449#issuecomment-3318182715)
Needs more though, closing for now.
(https://github.com/bitcoin/bitcoin/pull/33449#issuecomment-3318182715)
Needs more though, closing for now.
π¬ sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2367627572)
I added a commit to the end of this PR that should fix this issue. As I was working on this, I was thinking about your comment above about reworking the TRUC checks to just grab the clusters it needs directly from txgraph. The issue with that in the way our code works right now is that we can't fully realize our staging clusters until we've added all the transactions, applied dependencies, and staged all the RBF removals.
If we went ahead and imposed a topology requirement on transaction pa
...
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2367627572)
I added a commit to the end of this PR that should fix this issue. As I was working on this, I was thinking about your comment above about reworking the TRUC checks to just grab the clusters it needs directly from txgraph. The issue with that in the way our code works right now is that we can't fully realize our staging clusters until we've added all the transactions, applied dependencies, and staged all the RBF removals.
If we went ahead and imposed a topology requirement on transaction pa
...