π€ FranciscoBelchior reviewed a pull request: "util: Revert "common: Close non-std fds before exec in RunCommandJSON""
(https://github.com/bitcoin/bitcoin/pull/33063#pullrequestreview-3302569087)
Concluiu
(https://github.com/bitcoin/bitcoin/pull/33063#pullrequestreview-3302569087)
Concluiu
π¬ FranciscoBelchior commented on pull request "run_command: Close non-std fds when execing slave processes":
(https://github.com/bitcoin/bitcoin/pull/30756#discussion_r2404699387)
I didn't authorize this
(https://github.com/bitcoin/bitcoin/pull/30756#discussion_r2404699387)
I didn't authorize this
β
pinheadmz closed an issue: "Set default datacarriersize to 160 bytes"
(https://github.com/bitcoin/bitcoin/issues/33543)
(https://github.com/bitcoin/bitcoin/issues/33543)
π¬ pinheadmz commented on issue "Set default datacarriersize to 160 bytes":
(https://github.com/bitcoin/bitcoin/issues/33543#issuecomment-3369397436)
This is a duplicate of issues #33298 and #33240 Not to mention extensive discussion in pull requests #32381 #32359 #32406 and the meta discussion https://github.com/bitcoin-core/meta/discussions/18 and also the [mailing list post](https://groups.google.com/g/bitcoindev/c/d6ZO7gXGYbQ/m/mJyek28lDAAJ).
Additional op_return concerns were already addressed at length [here](https://delvingbitcoin.org/t/addressing-community-concerns-and-objections-regarding-my-recent-proposal-to-relax-bitcoin-cores-st
...
(https://github.com/bitcoin/bitcoin/issues/33543#issuecomment-3369397436)
This is a duplicate of issues #33298 and #33240 Not to mention extensive discussion in pull requests #32381 #32359 #32406 and the meta discussion https://github.com/bitcoin-core/meta/discussions/18 and also the [mailing list post](https://groups.google.com/g/bitcoindev/c/d6ZO7gXGYbQ/m/mJyek28lDAAJ).
Additional op_return concerns were already addressed at length [here](https://delvingbitcoin.org/t/addressing-community-concerns-and-objections-regarding-my-recent-proposal-to-relax-bitcoin-cores-st
...
β οΈ mikekelly opened an issue: "v30rc2 createrawtransaction unable to create txns with multiple OP_RETURNs"
(https://github.com/bitcoin/bitcoin/issues/33544)
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current behaviour
createrawtransaction in v30rc2 seems unable to create txns with multiple OP_RETURNs
### Expected behaviour
Expected v30 cli to be able to generate transactions with multiple OP_RETURN outputs.
### Steps to reproduce
```bash
$ bitcoin-cli -regtest createrawtransaction \
'[{"txid":"0000000000000000000000000000000000000000000000000000000000000000","vout":0}]' \
'[{"data":"68656c6c6f"}
...
(https://github.com/bitcoin/bitcoin/issues/33544)
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current behaviour
createrawtransaction in v30rc2 seems unable to create txns with multiple OP_RETURNs
### Expected behaviour
Expected v30 cli to be able to generate transactions with multiple OP_RETURN outputs.
### Steps to reproduce
```bash
$ bitcoin-cli -regtest createrawtransaction \
'[{"txid":"0000000000000000000000000000000000000000000000000000000000000000","vout":0}]' \
'[{"data":"68656c6c6f"}
...
β
cedwies closed a pull request: "tests: cover abortrescan() in-flight True path with dynamic-tail retry"
(https://github.com/bitcoin/bitcoin/pull/33131)
(https://github.com/bitcoin/bitcoin/pull/33131)
π¬ cedwies commented on pull request "tests: cover abortrescan() in-flight True path with dynamic-tail retry":
(https://github.com/bitcoin/bitcoin/pull/33131#issuecomment-3369403934)
Closing: asserting the β`abortrescan()` returns `True` while a rescan is in flightβ path is inherently racy in a functional test. The rescan runs in a separate thread and, on slow CI machines, can finish before the abort is observed, causing intermittent failures. Even with long rescans and retries thereβs a non-zero flake window. Thanks @maflcko for the review and feedback.
(https://github.com/bitcoin/bitcoin/pull/33131#issuecomment-3369403934)
Closing: asserting the β`abortrescan()` returns `True` while a rescan is in flightβ path is inherently racy in a functional test. The rescan runs in a separate thread and, on slow CI machines, can finish before the abort is observed, causing intermittent failures. Even with long rescans and retries thereβs a non-zero flake window. Thanks @maflcko for the review and feedback.
π pablomartin4btc approved a pull request: "refactor: unify container presence checks"
(https://github.com/bitcoin/bitcoin/pull/33192#pullrequestreview-3302612544)
re-ACK d870caca33ac9f0fdd76969a7341535c5722417e
In 2nd commit (b25879394ed1d6182d123787e8c03ae38bf884d1) did you miss the counts changes on `src/rpc/client.cpp` or was it intentional? _(Sorry if I miss a reason)_.
https://github.com/l0rinc/bitcoin/blob/f70d2c7faa8f7d724e146e4c409de9c6778b7299/src/rpc/client.cpp#L332-L343
(https://github.com/bitcoin/bitcoin/pull/33192#pullrequestreview-3302612544)
re-ACK d870caca33ac9f0fdd76969a7341535c5722417e
In 2nd commit (b25879394ed1d6182d123787e8c03ae38bf884d1) did you miss the counts changes on `src/rpc/client.cpp` or was it intentional? _(Sorry if I miss a reason)_.
https://github.com/l0rinc/bitcoin/blob/f70d2c7faa8f7d724e146e4c409de9c6778b7299/src/rpc/client.cpp#L332-L343
π¬ 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?