💬 maflcko commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2733252346)
As the test failures under wine happen intermittently, I expect it would be hard to debug and fix it upstream? Maybe someone can reproduce it in `rr`?
In any case, I don't mind if people maintain Wine compatibility, or even add it back to the normal CI, once Wine is fixed. However, until then, maintaining Wine compatibility is something to be done in a "nightly CI", with only fixes provided to this repo.
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2733252346)
As the test failures under wine happen intermittently, I expect it would be hard to debug and fix it upstream? Maybe someone can reproduce it in `rr`?
In any case, I don't mind if people maintain Wine compatibility, or even add it back to the normal CI, once Wine is fixed. However, until then, maintaining Wine compatibility is something to be done in a "nightly CI", with only fixes provided to this repo.
💬 maflcko commented on pull request "test: replace assert with assert_equal and assert_greater_than":
(https://github.com/bitcoin/bitcoin/pull/32091#discussion_r2001077998)
https://github.com/bitcoin/bitcoin/actions/runs/13923574432/job/38962671641?pr=32091#step:6:7169
failed
(https://github.com/bitcoin/bitcoin/pull/32091#discussion_r2001077998)
https://github.com/bitcoin/bitcoin/actions/runs/13923574432/job/38962671641?pr=32091#step:6:7169
failed
💬 Chand-ra commented on pull request "test: replace assert with assert_equal and assert_greater_than":
(https://github.com/bitcoin/bitcoin/pull/32091#discussion_r2001094779)
I ran the entire test suite from my `build` directory and the modified test passed just fine there. Any idea on what might cause the discrepancy?
(https://github.com/bitcoin/bitcoin/pull/32091#discussion_r2001094779)
I ran the entire test suite from my `build` directory and the modified test passed just fine there. Any idea on what might cause the discrepancy?
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2733350025)
Rebased.
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2733350025)
Rebased.
💬 hodlinator commented on pull request "net: Block v2->v1 transport downgrade if !fNetworkActive":
(https://github.com/bitcoin/bitcoin/pull/32073#discussion_r2001138653)
> Not blocking, but this seemed slightly more correct to me before, since now if `fNetworkActive == false` when caching the result, but `true` when disconnecting, we might not attempt v1 reconnect when we should have.
The probability that someone will be able to toggle `fNetworkActive` `false -> true` between the first loop starting and the second loop, when one also happens to have a peer in the right step in the handshake.. just doesn't feel like it should be something we need to support. B
...
(https://github.com/bitcoin/bitcoin/pull/32073#discussion_r2001138653)
> Not blocking, but this seemed slightly more correct to me before, since now if `fNetworkActive == false` when caching the result, but `true` when disconnecting, we might not attempt v1 reconnect when we should have.
The probability that someone will be able to toggle `fNetworkActive` `false -> true` between the first loop starting and the second loop, when one also happens to have a peer in the right step in the handshake.. just doesn't feel like it should be something we need to support. B
...
💬 ismaelsadeeq commented on pull request "Fee Estimation: Ignore all transactions that are CPFP'd":
(https://github.com/bitcoin/bitcoin/pull/30079#issuecomment-2733395488)
> This is a GitHub bug, but they are not going to fix it. The only way to fix it is by doing another rebase (or for a GitHub staff to manually clean it). See [maflcko/DrahtBot#43 (comment)](https://github.com/maflcko/DrahtBot/issues/43#issuecomment-2653314333)
I rebase against master and force-push with newer commits, it is still not updating https://github.com/bitcoin/bitcoin/compare/master...ismaelsadeeq:bitcoin:05-2023-ignore-transactions-with-parents
(https://github.com/bitcoin/bitcoin/pull/30079#issuecomment-2733395488)
> This is a GitHub bug, but they are not going to fix it. The only way to fix it is by doing another rebase (or for a GitHub staff to manually clean it). See [maflcko/DrahtBot#43 (comment)](https://github.com/maflcko/DrahtBot/issues/43#issuecomment-2653314333)
I rebase against master and force-push with newer commits, it is still not updating https://github.com/bitcoin/bitcoin/compare/master...ismaelsadeeq:bitcoin:05-2023-ignore-transactions-with-parents
💬 ismaelsadeeq commented on issue "wallet: rpc: `settxfee` sets the wallet feerate not fee":
(https://github.com/bitcoin/bitcoin/issues/31088#issuecomment-2733423213)
> We can look at what changes are needed, update the ones that don't break anything (e.g. rpc that simply give information but don't take a feerate as an argument).
This makes sense, currently internally `CFeeRate` is represented as feerate in `BTC/kvB`, although the newer replacement to it `FeeFrac` is going to be in `s/vb` see https://github.com/bitcoin/bitcoin/pull/31363/commits/8d1bbafa84bbca0e412f939823fa1a30ef839951
It will makes to first analyse how many or too critical of a change th
...
(https://github.com/bitcoin/bitcoin/issues/31088#issuecomment-2733423213)
> We can look at what changes are needed, update the ones that don't break anything (e.g. rpc that simply give information but don't take a feerate as an argument).
This makes sense, currently internally `CFeeRate` is represented as feerate in `BTC/kvB`, although the newer replacement to it `FeeFrac` is going to be in `s/vb` see https://github.com/bitcoin/bitcoin/pull/31363/commits/8d1bbafa84bbca0e412f939823fa1a30ef839951
It will makes to first analyse how many or too critical of a change th
...
💬 ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2733453010)
Rebased 8ef8f799b04eaea9091ed339dfe2cdb3a31ec245 -> 4e265debdc0319bbfcea915d9026b33810b810f8 ([`pr/subtree.24`](https://github.com/ryanofsky/bitcoin/commits/pr/subtree.24) -> [`pr/subtree.25`](https://github.com/ryanofsky/bitcoin/commits/pr/subtree.25), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/subtree.24-rebase..pr/subtree.25)) with no changes since #30975 has a merge conflict https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2732019454 and windows CI job was also faili
...
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2733453010)
Rebased 8ef8f799b04eaea9091ed339dfe2cdb3a31ec245 -> 4e265debdc0319bbfcea915d9026b33810b810f8 ([`pr/subtree.24`](https://github.com/ryanofsky/bitcoin/commits/pr/subtree.24) -> [`pr/subtree.25`](https://github.com/ryanofsky/bitcoin/commits/pr/subtree.25), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/subtree.24-rebase..pr/subtree.25)) with no changes since #30975 has a merge conflict https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2732019454 and windows CI job was also faili
...
🤔 TheCharlatan reviewed a pull request: "multiprocess: Add libmultiprocess git subtree"
(https://github.com/bitcoin/bitcoin/pull/31741#pullrequestreview-2694724554)
lgtm, but would you like people to review #31992 first?
(https://github.com/bitcoin/bitcoin/pull/31741#pullrequestreview-2694724554)
lgtm, but would you like people to review #31992 first?
💬 TheCharlatan commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r2001144249)
Maybe add `--numeric-owner` here as suggested in https://reproducible-builds.org/docs/archives/ (and as already done further down in this file).
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r2001144249)
Maybe add `--numeric-owner` here as suggested in https://reproducible-builds.org/docs/archives/ (and as already done further down in this file).
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2733485415)
The feedback from @hodlinator and @laanwj has been addressed.
> Lingering Qt5 mentions
Removed.
> Could mention in commit message why CRB repo needed to be enabled in ci/test/01_base_install.sh?
Added a comment.
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2733485415)
The feedback from @hodlinator and @laanwj has been addressed.
> Lingering Qt5 mentions
Removed.
> Could mention in commit message why CRB repo needed to be enabled in ci/test/01_base_install.sh?
Added a comment.
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2001211668)
Thanks! [Taken](https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2733485415).
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2001211668)
Thanks! [Taken](https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2733485415).
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2001212035)
Thanks! [Taken](https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2733485415).
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2001212035)
Thanks! [Taken](https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2733485415).
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2001213058)
Thanks! [Taken](https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2733485415).
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2001213058)
Thanks! [Taken](https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2733485415).
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2001219461)
This code is designed to skip compiling unneeded plugins. You can verify this by checking the build logs.
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2001219461)
This code is designed to skip compiling unneeded plugins. You can verify this by checking the build logs.
💬 hodlinator commented on pull request "http: Make server shutdown more robust":
(https://github.com/bitcoin/bitcoin/pull/31929#issuecomment-2733522421)
Thank you for providing your perspective @laanwj!
In working on this PR, I've also gained an [appreciation for getting rid of libevent](https://github.com/bitcoin/bitcoin/issues/31894#issuecomment-2678077496). However, I'm not sure when that will happen, maybe not until post-30.0. In the meantime we are seeing issues on CI such as #31894 ([analysis](https://github.com/bitcoin/bitcoin/issues/31894#issuecomment-2666012863)).
What this PR is doing is adding our own request ids to track which
...
(https://github.com/bitcoin/bitcoin/pull/31929#issuecomment-2733522421)
Thank you for providing your perspective @laanwj!
In working on this PR, I've also gained an [appreciation for getting rid of libevent](https://github.com/bitcoin/bitcoin/issues/31894#issuecomment-2678077496). However, I'm not sure when that will happen, maybe not until post-30.0. In the meantime we are seeing issues on CI such as #31894 ([analysis](https://github.com/bitcoin/bitcoin/issues/31894#issuecomment-2666012863)).
What this PR is doing is adding our own request ids to track which
...
💬 yancyribbens commented on pull request "wallet: add coin selection parameter `add_excess_to_recipient_position` for changeless txs with excess that would be added to fees":
(https://github.com/bitcoin/bitcoin/pull/30080#issuecomment-2733538448)
This PR looks interesting from the idea that throw away change can go to a different party instead of just throwing away to fees. It first glance though, it seems like the implementation is confusing. For example, I don't understand what `max_excess` is needed for.
From the commit message:
> If set, excess from changeless spends can not exceed the lesser of this amount and the current cost_of_change, otherwise just use cost_of_change by default
This doesn't explain _why_ this parameter
...
(https://github.com/bitcoin/bitcoin/pull/30080#issuecomment-2733538448)
This PR looks interesting from the idea that throw away change can go to a different party instead of just throwing away to fees. It first glance though, it seems like the implementation is confusing. For example, I don't understand what `max_excess` is needed for.
From the commit message:
> If set, excess from changeless spends can not exceed the lesser of this amount and the current cost_of_change, otherwise just use cost_of_change by default
This doesn't explain _why_ this parameter
...
✅ maflcko closed a pull request: "Fee Estimation: Ignore all transactions that are CPFP'd"
(https://github.com/bitcoin/bitcoin/pull/30079)
(https://github.com/bitcoin/bitcoin/pull/30079)
💬 maflcko commented on pull request "Fee Estimation: Ignore all transactions that are CPFP'd":
(https://github.com/bitcoin/bitcoin/pull/30079#issuecomment-2733545178)
Sorry, I wanted to close and re-open, but that won't work, because of the push. Can you restore the last synced push and then re-open and re-try the rebase?
(https://github.com/bitcoin/bitcoin/pull/30079#issuecomment-2733545178)
Sorry, I wanted to close and re-open, but that won't work, because of the push. Can you restore the last synced push and then re-open and re-try the rebase?
💬 Sjors commented on issue "Stratum v2 via IPC Mining Interface tracking issue":
(https://github.com/bitcoin/bitcoin/issues/31098#issuecomment-2733571093)
More things have been merged... for build system fans I recommend #31992, for interface fans #31785.
(https://github.com/bitcoin/bitcoin/issues/31098#issuecomment-2733571093)
More things have been merged... for build system fans I recommend #31992, for interface fans #31785.