💬 l0rinc commented on pull request "refactor: unify container presence checks":
(https://github.com/bitcoin/bitcoin/pull/33192#issuecomment-3369524605)
thanks for the reviews. I have reverted conflicting changes. It's better to be lightweight and not conflict with other PRs, it's not critical to be exhaustive with the replaces just that the ones we have are correct.
(https://github.com/bitcoin/bitcoin/pull/33192#issuecomment-3369524605)
thanks for the reviews. I have reverted conflicting changes. It's better to be lightweight and not conflict with other PRs, it's not critical to be exhaustive with the replaces just that the ones we have are correct.
⚠️ hebasto opened an issue: "Implicit conversion from `fs::path` to `std::string` when constructing file streams"
(https://github.com/bitcoin/bitcoin/issues/33545)
When building with libc++ that has LWG 3430 [implemented](https://github.com/llvm/llvm-project/commit/4761e74a276ee1f38596f4849daa9d633929f2ae), constructing `std::ifstream` or `std::ofstream` from an `fs::path` object triggers its implicit conversion to `std::string`. This behavior has been undesirable ever since `fs::path` was introduced. See, for example: https://github.com/bitcoin/bitcoin/blob/a33bd767a37dccf39a094d03c2f62ea81633410f/src/util/fs.h#L54-L55
(https://github.com/bitcoin/bitcoin/issues/33545)
When building with libc++ that has LWG 3430 [implemented](https://github.com/llvm/llvm-project/commit/4761e74a276ee1f38596f4849daa9d633929f2ae), constructing `std::ifstream` or `std::ofstream` from an `fs::path` object triggers its implicit conversion to `std::string`. This behavior has been undesirable ever since `fs::path` was introduced. See, for example: https://github.com/bitcoin/bitcoin/blob/a33bd767a37dccf39a094d03c2f62ea81633410f/src/util/fs.h#L54-L55
💬 hebasto commented on issue "Implicit conversion from `fs::path` to `std::string` when constructing file streams":
(https://github.com/bitcoin/bitcoin/issues/33545#issuecomment-3369553341)
cc @ryanofsky
(https://github.com/bitcoin/bitcoin/issues/33545#issuecomment-3369553341)
cc @ryanofsky
💬 hebasto commented on pull request "Add coins (UTXOs) tab and makes it view-only":
(https://github.com/bitcoin-core/gui/pull/898#issuecomment-3369556757)
cc @achow101 @Sjors
(https://github.com/bitcoin-core/gui/pull/898#issuecomment-3369556757)
cc @achow101 @Sjors
🤔 pablomartin4btc reviewed a pull request: "Clear out space on centos job"
(https://github.com/bitcoin/bitcoin/pull/33514#pullrequestreview-3302631883)
<details>
<summary>tACK 014b9f6268390b78e66e1be24813d3f069588442</summary>
```
Space freed: 22GB (23654664KB)
```
<img width="617" height="498" alt="Image" src="https://github.com/user-attachments/assets/817d214b-4f93-4590-9f66-b36b12477b15" />
https://github.com/pablomartin4btc/bitcoin/actions/runs/18266112018/job/52000630402
</details>
It happened to me a couple of days ago: https://github.com/pablomartin4btc/bitcoin/actions/runs/18234273673/job/51924700571
(https://github.com/bitcoin/bitcoin/pull/33514#pullrequestreview-3302631883)
<details>
<summary>tACK 014b9f6268390b78e66e1be24813d3f069588442</summary>
```
Space freed: 22GB (23654664KB)
```
<img width="617" height="498" alt="Image" src="https://github.com/user-attachments/assets/817d214b-4f93-4590-9f66-b36b12477b15" />
https://github.com/pablomartin4btc/bitcoin/actions/runs/18266112018/job/52000630402
</details>
It happened to me a couple of days ago: https://github.com/pablomartin4btc/bitcoin/actions/runs/18234273673/job/51924700571
📝 theStack opened a pull request: "test: add functional test for `TestShell` (matching doc example)"
(https://github.com/bitcoin/bitcoin/pull/33546)
This PR adds a functional framework test for the `TestShell` class. The primary motivation for this is to avoid that the interactive instructions for the interactive Python shell in `test-shell.md` get outdated or broken without noticing, a problem we had already several times in the past (see #26520, #27906, #31415). Having a copy is still not perfect, as docs and functional test have to be kept in sync, but I don't expect this to be a problem in practice, assuming the hint in the functional te
...
(https://github.com/bitcoin/bitcoin/pull/33546)
This PR adds a functional framework test for the `TestShell` class. The primary motivation for this is to avoid that the interactive instructions for the interactive Python shell in `test-shell.md` get outdated or broken without noticing, a problem we had already several times in the past (see #26520, #27906, #31415). Having a copy is still not perfect, as docs and functional test have to be kept in sync, but I don't expect this to be a problem in practice, assuming the hint in the functional te
...
💬 mikekelly commented on issue "v30rc2 createrawtransaction unable to create txns with multiple OP_RETURNs":
(https://github.com/bitcoin/bitcoin/issues/33544#issuecomment-3370003020)
Here's the offending lines
https://github.com/bitcoin/bitcoin/blob/30.x/src/rpc/rawtransaction_util.cpp#L109-L111
(https://github.com/bitcoin/bitcoin/issues/33544#issuecomment-3370003020)
Here's the offending lines
https://github.com/bitcoin/bitcoin/blob/30.x/src/rpc/rawtransaction_util.cpp#L109-L111
💬 maflcko commented on issue "v30rc2 createrawtransaction unable to create txns with multiple OP_RETURNs":
(https://github.com/bitcoin/bitcoin/issues/33544#issuecomment-3370064859)
> ### Is there an existing issue for this?
>
> * [x] I have searched the existing issues
See #32478?
(https://github.com/bitcoin/bitcoin/issues/33544#issuecomment-3370064859)
> ### Is there an existing issue for this?
>
> * [x] I have searched the existing issues
See #32478?
💬 maflcko commented on pull request "test: add functional test for `TestShell` (matching doc example)":
(https://github.com/bitcoin/bitcoin/pull/33546#issuecomment-3370068826)
Looks like the new test times out in CI?
(https://github.com/bitcoin/bitcoin/pull/33546#issuecomment-3370068826)
Looks like the new test times out in CI?
💬 mikekelly commented on issue "v30rc2 createrawtransaction unable to create txns with multiple OP_RETURNs":
(https://github.com/bitcoin/bitcoin/issues/33544#issuecomment-3370099415)
@maflcko - apologies. Out of interest, is it a hard rule that RPC changes like this must follow a release later? Seems strange to relax standardness but not give users a straightforward way to make use of it
(https://github.com/bitcoin/bitcoin/issues/33544#issuecomment-3370099415)
@maflcko - apologies. Out of interest, is it a hard rule that RPC changes like this must follow a release later? Seems strange to relax standardness but not give users a straightforward way to make use of it
✅ mikekelly closed an issue: "v30rc2 createrawtransaction unable to create txns with multiple OP_RETURNs"
(https://github.com/bitcoin/bitcoin/issues/33544)
(https://github.com/bitcoin/bitcoin/issues/33544)
💬 maflcko commented on issue "Implicit conversion from `fs::path` to `std::string` when constructing file streams":
(https://github.com/bitcoin/bitcoin/issues/33545#issuecomment-3370123776)
I guess this means any fs::path would have to be explicitly cast to a std::fs::path, which seems fine to do. I guess you are asking how to deal with the fs linter?
(https://github.com/bitcoin/bitcoin/issues/33545#issuecomment-3370123776)
I guess this means any fs::path would have to be explicitly cast to a std::fs::path, which seems fine to do. I guess you are asking how to deal with the fs linter?
💬 apogio commented on pull request "Add coins (UTXOs) tab and makes it view-only":
(https://github.com/bitcoin-core/gui/pull/898#issuecomment-3370131703)
So, I've got 3 failed pipeline checks. I'll need to check them.
(https://github.com/bitcoin-core/gui/pull/898#issuecomment-3370131703)
So, I've got 3 failed pipeline checks. I'll need to check them.
💬 maflcko commented on issue "v30rc2 createrawtransaction unable to create txns with multiple OP_RETURNs":
(https://github.com/bitcoin/bitcoin/issues/33544#issuecomment-3370140629)
With policy changes, the expectation is that one release is needed to upgrade a sufficient portion of the network, until the wallet and RPC can offer them as features. Otherwise, users may not be able to propagate such transactions, or only intermittently.
(https://github.com/bitcoin/bitcoin/issues/33544#issuecomment-3370140629)
With policy changes, the expectation is that one release is needed to upgrade a sufficient portion of the network, until the wallet and RPC can offer them as features. Otherwise, users may not be able to propagate such transactions, or only intermittently.
⚠️ maflcko reopened an issue: "ARM Windows build and release"
(https://github.com/bitcoin/bitcoin/issues/31388)
This was brought up recently:
* https://bitcointalk.org/index.php?topic=5517601.0
* https://groups.google.com/d/msgid/bitcoindev/e5b06aaa-1fe9-4c8f-a0ea-db10f8a7e48cn%40googlegroups.com
(https://github.com/bitcoin/bitcoin/issues/31388)
This was brought up recently:
* https://bitcointalk.org/index.php?topic=5517601.0
* https://groups.google.com/d/msgid/bitcoindev/e5b06aaa-1fe9-4c8f-a0ea-db10f8a7e48cn%40googlegroups.com
💬 maflcko commented on issue "ARM Windows build and release":
(https://github.com/bitcoin/bitcoin/issues/31388#issuecomment-3370158680)
I guess we could vendor the toolchain in guix ourselves, so re-opening for now. Though, I am not sure if we should vendor it ourselves.
(https://github.com/bitcoin/bitcoin/issues/31388#issuecomment-3370158680)
I guess we could vendor the toolchain in guix ourselves, so re-opening for now. Though, I am not sure if we should vendor it ourselves.
💬 mikekelly commented on issue "v30rc2 createrawtransaction unable to create txns with multiple OP_RETURNs":
(https://github.com/bitcoin/bitcoin/issues/33544#issuecomment-3370161045)
@maflcko genuine question - why couldn't the policy be to accommodate policy changes in the same release but with a warning to users about potential propagation issues (and then remove it in the next release)?
(https://github.com/bitcoin/bitcoin/issues/33544#issuecomment-3370161045)
@maflcko genuine question - why couldn't the policy be to accommodate policy changes in the same release but with a warning to users about potential propagation issues (and then remove it in the next release)?
💬 maflcko commented on issue "v30rc2 createrawtransaction unable to create txns with multiple OP_RETURNs":
(https://github.com/bitcoin/bitcoin/issues/33544#issuecomment-3370168020)
Yeah, it is always a bit of a case-by-case decision. However, I don't think a pull request in a reviewable state exists to implement this? So nothing can be merged into 31.x, nor 30.x anyway.
(https://github.com/bitcoin/bitcoin/issues/33544#issuecomment-3370168020)
Yeah, it is always a bit of a case-by-case decision. However, I don't think a pull request in a reviewable state exists to implement this? So nothing can be merged into 31.x, nor 30.x anyway.
💬 BenWestgate commented on pull request "doc: update multisig tutorial to use multipath descriptors":
(https://github.com/bitcoin/bitcoin/pull/33286#discussion_r2405153857)
I updated the note with your feedback and implemented the comment nit in the test. Does this address the confusion?
> Note that previously at least two descriptors were usually used, one for external derivation paths and one for internal ones. Since https://github.com/bitcoin/bitcoin/pull/22838 this redundancy has been eliminated by a multipath descriptor with <code><0;1></code> at the [BIP-44](https://github.com/bitcoin/bips/blob/master/bip-0044.mediawiki#change) change level expanding to e
...
(https://github.com/bitcoin/bitcoin/pull/33286#discussion_r2405153857)
I updated the note with your feedback and implemented the comment nit in the test. Does this address the confusion?
> Note that previously at least two descriptors were usually used, one for external derivation paths and one for internal ones. Since https://github.com/bitcoin/bitcoin/pull/22838 this redundancy has been eliminated by a multipath descriptor with <code><0;1></code> at the [BIP-44](https://github.com/bitcoin/bips/blob/master/bip-0044.mediawiki#change) change level expanding to e
...
👍 hodlinator approved a pull request: "Clear out space on centos job"
(https://github.com/bitcoin/bitcoin/pull/33514#pullrequestreview-3303220896)
ACK 014b9f6268390b78e66e1be24813d3f069588442
Was curious if it would take less time to switch to the *stream10-minimal container*, but it seems to come with the same amount of */usr/local/lib/android* files, taking over 3min to remove regardless (https://github.com/hodlinator/bitcoin/actions/runs/18273895919/job/52021497688#step:6:249).
(https://github.com/bitcoin/bitcoin/pull/33514#pullrequestreview-3303220896)
ACK 014b9f6268390b78e66e1be24813d3f069588442
Was curious if it would take less time to switch to the *stream10-minimal container*, but it seems to come with the same amount of */usr/local/lib/android* files, taking over 3min to remove regardless (https://github.com/hodlinator/bitcoin/actions/runs/18273895919/job/52021497688#step:6:249).