💬 hebasto commented on issue "build: build broken with older supported Boost":
(https://github.com/bitcoin/bitcoin/issues/34101#issuecomment-3670300267)
Is it a good time to consider bumping of the Boost minimum supported version?
(https://github.com/bitcoin/bitcoin/issues/34101#issuecomment-3670300267)
Is it a good time to consider bumping of the Boost minimum supported version?
💬 0xB10C commented on pull request "test: address self-announcement":
(https://github.com/bitcoin/bitcoin/pull/34039#discussion_r2631122111)
done.
(https://github.com/bitcoin/bitcoin/pull/34039#discussion_r2631122111)
done.
💬 0xB10C commented on pull request "test: address self-announcement":
(https://github.com/bitcoin/bitcoin/pull/34039#discussion_r2631122670)
Dropped it.
(https://github.com/bitcoin/bitcoin/pull/34039#discussion_r2631122670)
Dropped it.
💬 0xB10C commented on pull request "test: address self-announcement":
(https://github.com/bitcoin/bitcoin/pull/34039#issuecomment-3670317642)
Addressed comments by mzumsande
(https://github.com/bitcoin/bitcoin/pull/34039#issuecomment-3670317642)
Addressed comments by mzumsande
💬 fanquake commented on issue "build: build broken with older supported Boost":
(https://github.com/bitcoin/bitcoin/issues/34101#issuecomment-3670335345)
Looks like it might have been added in Boost 1.78.0: https://github.com/boostorg/multi_index/issues/35.
(https://github.com/bitcoin/bitcoin/issues/34101#issuecomment-3670335345)
Looks like it might have been added in Boost 1.78.0: https://github.com/boostorg/multi_index/issues/35.
💬 maflcko commented on issue "build: build broken with older supported Boost":
(https://github.com/bitcoin/bitcoin/issues/34101#issuecomment-3670376266)
> Is it a good time to consider bumping of the Boost minimum supported version?
I think there are users out there on Ubuntu:22.04 and Debian Bookworm, so breaking them seems a bit too early?
(https://github.com/bitcoin/bitcoin/issues/34101#issuecomment-3670376266)
> Is it a good time to consider bumping of the Boost minimum supported version?
I think there are users out there on Ubuntu:22.04 and Debian Bookworm, so breaking them seems a bit too early?
💬 l0rinc commented on pull request "scripted-diff: [doc] Unify stale copyright headers":
(https://github.com/bitcoin/bitcoin/pull/34084#issuecomment-3670396535)
This is tangentially related to https://github.com/bitcoin/bitcoin/issues/29445 - I don't really mind either way, I just want to stop the stupid headers distracting code review.
I have reviewed the change by:
* validating the regex 👍
* scrolling mindlessly through https://github.com/bitcoin/bitcoin/pull/34084/files?diff=unified 👍
* Manually checking https://patch-diff.githubusercontent.com/raw/bitcoin/bitcoin/pull/34084.patch and asking LLMs to proofread it as well 👍
This is officia
...
(https://github.com/bitcoin/bitcoin/pull/34084#issuecomment-3670396535)
This is tangentially related to https://github.com/bitcoin/bitcoin/issues/29445 - I don't really mind either way, I just want to stop the stupid headers distracting code review.
I have reviewed the change by:
* validating the regex 👍
* scrolling mindlessly through https://github.com/bitcoin/bitcoin/pull/34084/files?diff=unified 👍
* Manually checking https://patch-diff.githubusercontent.com/raw/bitcoin/bitcoin/pull/34084.patch and asking LLMs to proofread it as well 👍
This is officia
...
📝 fanquake opened a pull request: "depends: capnp 1.3.0"
(https://github.com/bitcoin/bitcoin/pull/34102)
Update capnp in depends to `1.3.0`.
Changes: https://github.com/capnproto/capnproto/compare/release-1.2.0...release-1.3.0.
(https://github.com/bitcoin/bitcoin/pull/34102)
Update capnp in depends to `1.3.0`.
Changes: https://github.com/capnproto/capnproto/compare/release-1.2.0...release-1.3.0.
💬 l0rinc commented on issue "build: build broken with older supported Boost":
(https://github.com/bitcoin/bitcoin/issues/34101#issuecomment-3670429071)
We could revert these instances, but in that case we have to make sure the CI catches these next time.
Let me know if there's anything you'd like me to do here.
(https://github.com/bitcoin/bitcoin/issues/34101#issuecomment-3670429071)
We could revert these instances, but in that case we have to make sure the CI catches these next time.
Let me know if there's anything you'd like me to do here.
💬 marcofleon commented on issue "build: build broken with older supported Boost":
(https://github.com/bitcoin/bitcoin/issues/34101#issuecomment-3670432470)
> Looks like it might have been added in Boost 1.78.0
Just confirmed that 1.78.0 does build without errors.
(https://github.com/bitcoin/bitcoin/issues/34101#issuecomment-3670432470)
> Looks like it might have been added in Boost 1.78.0
Just confirmed that 1.78.0 does build without errors.
💬 maflcko commented on issue "build: build broken with older supported Boost":
(https://github.com/bitcoin/bitcoin/issues/34101#issuecomment-3670454317)
> We could revert these instances, but in that case we have to make sure the CI catches these next time.
I don't think it will be possible that CI catches "everything". In theory, the `ci/test/00_setup_env_native_previous_releases.sh` could be modified to run without depends to catch this, but then we wouldn't be checking depends. So a new ci config would have to be added. But that again will only check either gcc, or clang. So a new ci config will have to be added for both....
(https://github.com/bitcoin/bitcoin/issues/34101#issuecomment-3670454317)
> We could revert these instances, but in that case we have to make sure the CI catches these next time.
I don't think it will be possible that CI catches "everything". In theory, the `ci/test/00_setup_env_native_previous_releases.sh` could be modified to run without depends to catch this, but then we wouldn't be checking depends. So a new ci config would have to be added. But that again will only check either gcc, or clang. So a new ci config will have to be added for both....
💬 instagibbs commented on pull request "policy: Remove individual transaction <minrelay restriction":
(https://github.com/bitcoin/bitcoin/pull/33892#discussion_r2631270950)
This is already the case if a transaction is >=minrelay but not reaching floating minfee, so it's an existing footgun unfortunately.
It started out as experimental support before 1P1C was the scaled down idea IIRC, so we probably should have come back and restricted it as soon as that direction was chosen.
Willing to discuss this in an open issue somewhere to see what should be done if anything.
(https://github.com/bitcoin/bitcoin/pull/33892#discussion_r2631270950)
This is already the case if a transaction is >=minrelay but not reaching floating minfee, so it's an existing footgun unfortunately.
It started out as experimental support before 1P1C was the scaled down idea IIRC, so we probably should have come back and restricted it as soon as that direction was chosen.
Willing to discuss this in an open issue somewhere to see what should be done if anything.
💬 stringintech commented on pull request "kernel: Add block header support and validation":
(https://github.com/bitcoin/bitcoin/pull/33822#discussion_r2631281218)
Seems for len > 80 we would not throw though (please see https://github.com/bitcoin/bitcoin/pull/33822#discussion_r2623386830).
(https://github.com/bitcoin/bitcoin/pull/33822#discussion_r2631281218)
Seems for len > 80 we would not throw though (please see https://github.com/bitcoin/bitcoin/pull/33822#discussion_r2623386830).
💬 instagibbs commented on pull request "policy: allow <minrelay txns in package context if paid for by cpfp":
(https://github.com/bitcoin/bitcoin/pull/33892#discussion_r2631285602)
re: the release note, something like:
```
diff --git a/doc/release-notes-33892.md b/doc/release-notes-33892.md
index 125ee4a51f..5f51336e5b 100644
--- a/doc/release-notes-33892.md
+++ b/doc/release-notes-33892.md
@@ -1,5 +1,6 @@
P2P and network changes
-----------------------
- All transaction versions including non-TRUC can now be relayed when below minrelay,
- including 0 fee, as long as the package feerate is above the floating minfee (#33892)
+ including 0 fee, as lo
...
(https://github.com/bitcoin/bitcoin/pull/33892#discussion_r2631285602)
re: the release note, something like:
```
diff --git a/doc/release-notes-33892.md b/doc/release-notes-33892.md
index 125ee4a51f..5f51336e5b 100644
--- a/doc/release-notes-33892.md
+++ b/doc/release-notes-33892.md
@@ -1,5 +1,6 @@
P2P and network changes
-----------------------
- All transaction versions including non-TRUC can now be relayed when below minrelay,
- including 0 fee, as long as the package feerate is above the floating minfee (#33892)
+ including 0 fee, as lo
...
💬 stringintech commented on pull request "kernel: Add block header support and validation":
(https://github.com/bitcoin/bitcoin/pull/33822#discussion_r2631293264)
Just noticed a previous related reviewed comment: https://github.com/bitcoin/bitcoin/pull/33822#discussion_r2508035777
(https://github.com/bitcoin/bitcoin/pull/33822#discussion_r2631293264)
Just noticed a previous related reviewed comment: https://github.com/bitcoin/bitcoin/pull/33822#discussion_r2508035777
💬 vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-3670577096)
This is intended to work on CI and it does so well. Should I reintroduce [INTERNET_TRAFFIC_EXPECTED](https://github.com/bitcoin/bitcoin/pull/31349#discussion_r2333792206) to deal with local runs by making it possible to turn these reports into non-fatal errors?
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-3670577096)
This is intended to work on CI and it does so well. Should I reintroduce [INTERNET_TRAFFIC_EXPECTED](https://github.com/bitcoin/bitcoin/pull/31349#discussion_r2333792206) to deal with local runs by making it possible to turn these reports into non-fatal errors?
💬 l0rinc commented on pull request "miniscript refactor: Remove unique_ptr-indirection":
(https://github.com/bitcoin/bitcoin/pull/31713#issuecomment-3670582500)
Sorry for the conflict - happy to re-review after the rebase. Tried it locally, seems trivial.
(https://github.com/bitcoin/bitcoin/pull/31713#issuecomment-3670582500)
Sorry for the conflict - happy to re-review after the rebase. Tried it locally, seems trivial.
💬 fanquake commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-3670590566)
> This is intended to work on CI and it does so well.
Being able to run the CI locally is fully supported and a required use case (there are CI jobs which are not run in the main repo).
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-3670590566)
> This is intended to work on CI and it does so well.
Being able to run the CI locally is fully supported and a required use case (there are CI jobs which are not run in the main repo).
🤔 furszy reviewed a pull request: "rpc, doc: clarify the response of listtransactions RPC"
(https://github.com/bitcoin/bitcoin/pull/32737#pullrequestreview-3593328432)
ACK 1ed8e7616527c69dbaa9904cda59e3b73c29fa5d
(https://github.com/bitcoin/bitcoin/pull/32737#pullrequestreview-3593328432)
ACK 1ed8e7616527c69dbaa9904cda59e3b73c29fa5d
💬 maflcko commented on pull request "scripted-diff: [doc] Unify stale copyright headers":
(https://github.com/bitcoin/bitcoin/pull/34084#issuecomment-3670620730)
> Nit: if you need to touch again, please consider:
Thanks, addressed those that are in scope for the issues listed in the pull request description. The others seem unrelated and can be addressed in a different pull request either now or later, if there is need to.
(https://github.com/bitcoin/bitcoin/pull/34084#issuecomment-3670620730)
> Nit: if you need to touch again, please consider:
Thanks, addressed those that are in scope for the issues listed in the pull request description. The others seem unrelated and can be addressed in a different pull request either now or later, if there is need to.
🤔 rkrux reviewed a pull request: "doc: Use multipath descriptors in descriptors.md and linked test"
(https://github.com/bitcoin/bitcoin/pull/34100#pullrequestreview-3593332537)
Concept ACK 912d837326145c304662ca29a01ccd1d240946e8
Multi-path convention is indeed convenient and preferred.
(https://github.com/bitcoin/bitcoin/pull/34100#pullrequestreview-3593332537)
Concept ACK 912d837326145c304662ca29a01ccd1d240946e8
Multi-path convention is indeed convenient and preferred.