👍 hebasto approved a pull request: "ci: Allow build dir on CI host"
(https://github.com/bitcoin/bitcoin/pull/31428#pullrequestreview-2531869230)
re-ACK 8888ee4403fa62972c49e18752695d15fd32c0b0.
Only rebased since my recent [review](https://github.com/bitcoin/bitcoin/pull/31428#pullrequestreview-2517422562). Verified with:
```
git range-diff master fac0ab3286888c2eab674f6814ba8006e3a850a8 8888ee4403fa62972c49e18752695d15fd32c0b0
```
(https://github.com/bitcoin/bitcoin/pull/31428#pullrequestreview-2531869230)
re-ACK 8888ee4403fa62972c49e18752695d15fd32c0b0.
Only rebased since my recent [review](https://github.com/bitcoin/bitcoin/pull/31428#pullrequestreview-2517422562). Verified with:
```
git range-diff master fac0ab3286888c2eab674f6814ba8006e3a850a8 8888ee4403fa62972c49e18752695d15fd32c0b0
```
💬 hebasto commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2572844315)
Rebased on top of the https://github.com/bitcoin/bitcoin/pull/31428.
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2572844315)
Rebased on top of the https://github.com/bitcoin/bitcoin/pull/31428.
💬 maflcko commented on pull request "refactor: modernize recent `ByteType` usages and read/write functions":
(https://github.com/bitcoin/bitcoin/pull/31601#issuecomment-2572872762)
Also, for the second commit, it may be helpful to add the commit since the change is possible, like I did in f23d6a572d62ff640a28718ff0771c3f0f87f365, where I presume it is cherry-picked from?
(https://github.com/bitcoin/bitcoin/pull/31601#issuecomment-2572872762)
Also, for the second commit, it may be helpful to add the commit since the change is possible, like I did in f23d6a572d62ff640a28718ff0771c3f0f87f365, where I presume it is cherry-picked from?
💬 maflcko commented on pull request "ci: Bump centos stream 10":
(https://github.com/bitcoin/bitcoin/pull/31593#issuecomment-2572880648)
Yeah, it is blocked on https://bugzilla.redhat.com/show_bug.cgi?id=2335616.
Also, I am thinking it is fine to drop the 32-bit build, given that it is mostly redundant with ci/test/00_setup_env_i686_multiprocess.sh anyway.
(https://github.com/bitcoin/bitcoin/pull/31593#issuecomment-2572880648)
Yeah, it is blocked on https://bugzilla.redhat.com/show_bug.cgi?id=2335616.
Also, I am thinking it is fine to drop the 32-bit build, given that it is mostly redundant with ci/test/00_setup_env_i686_multiprocess.sh anyway.
📝 maflcko converted_to_draft a pull request: "ci: Bump centos stream 10"
(https://github.com/bitcoin/bitcoin/pull/31593)
This is a follow-up to fa47baa03bcfcf44fb2ed05f009a32d32f860c45, which bumped the gcc version to avoid a warning bloat in the CI log. However, it is also required to bump python3, see https://github.com/bitcoin/bitcoin/issues/31476#issue-2735206340
> This will uncover an issue in the centos task that the correct python version is missing. I guess this should be fixed by installing and activating an acceptable python version.
Instead of bumping the packages individually in centos stream 9,
...
(https://github.com/bitcoin/bitcoin/pull/31593)
This is a follow-up to fa47baa03bcfcf44fb2ed05f009a32d32f860c45, which bumped the gcc version to avoid a warning bloat in the CI log. However, it is also required to bump python3, see https://github.com/bitcoin/bitcoin/issues/31476#issue-2735206340
> This will uncover an issue in the centos task that the correct python version is missing. I guess this should be fixed by installing and activating an acceptable python version.
Instead of bumping the packages individually in centos stream 9,
...
🚀 fanquake merged a pull request: "txmempool: fix typos in comments"
(https://github.com/bitcoin/bitcoin/pull/31584)
(https://github.com/bitcoin/bitcoin/pull/31584)
💬 hebasto commented on pull request "doc: Clarify min macOS and Xcode version":
(https://github.com/bitcoin/bitcoin/pull/31608#discussion_r1904026312)
nit: While touching this line, we can use "+" or "and newer" consistently without mixing both.
(https://github.com/bitcoin/bitcoin/pull/31608#discussion_r1904026312)
nit: While touching this line, we can use "+" or "and newer" consistently without mixing both.
👍 hebasto approved a pull request: "doc: Clarify min macOS and Xcode version"
(https://github.com/bitcoin/bitcoin/pull/31608#pullrequestreview-2531924374)
ACK fa728e4f184ebbbcab07047cd8790fb9b16d5681.
(https://github.com/bitcoin/bitcoin/pull/31608#pullrequestreview-2531924374)
ACK fa728e4f184ebbbcab07047cd8790fb9b16d5681.
🤔 fanquake reviewed a pull request: "doc: Clarify min macOS and Xcode version"
(https://github.com/bitcoin/bitcoin/pull/31608#pullrequestreview-2531932011)
> only the latest security release is supported.
Should this change `OSX_MIN_VERSION`? Otherwise we are just changing the docs to no-longer reflect the compatibility of the binary we ship (it will still run on any 13.x).
(https://github.com/bitcoin/bitcoin/pull/31608#pullrequestreview-2531932011)
> only the latest security release is supported.
Should this change `OSX_MIN_VERSION`? Otherwise we are just changing the docs to no-longer reflect the compatibility of the binary we ship (it will still run on any 13.x).
💬 fanquake commented on pull request "doc: Clarify min macOS and Xcode version":
(https://github.com/bitcoin/bitcoin/pull/31608#discussion_r1904030950)
typo: `providing the at least the`
(https://github.com/bitcoin/bitcoin/pull/31608#discussion_r1904030950)
typo: `providing the at least the`
💬 maflcko commented on pull request "doc: Clarify min macOS and Xcode version":
(https://github.com/bitcoin/bitcoin/pull/31608#issuecomment-2572916602)
> > only the latest security release is supported.
>
> Should this change `OSX_MIN_VERSION`? Otherwise we are just changing the docs to no-longer reflect the compatibility of the binary we ship (it will still run on any 13.x).
While I think that only the latest minor security release is supported, it may be a bit too much handholding and churn to bump `OSX_MIN_VERSION` on every new macOS minor release. In reality it will take time to get into a release and until users upgrade, so I wonder
...
(https://github.com/bitcoin/bitcoin/pull/31608#issuecomment-2572916602)
> > only the latest security release is supported.
>
> Should this change `OSX_MIN_VERSION`? Otherwise we are just changing the docs to no-longer reflect the compatibility of the binary we ship (it will still run on any 13.x).
While I think that only the latest minor security release is supported, it may be a bit too much handholding and churn to bump `OSX_MIN_VERSION` on every new macOS minor release. In reality it will take time to get into a release and until users upgrade, so I wonder
...
🚀 fanquake merged a pull request: "depends: Update capnproto to 1.1.0"
(https://github.com/bitcoin/bitcoin/pull/31552)
(https://github.com/bitcoin/bitcoin/pull/31552)
💬 hebasto commented on pull request "qa: Ensure consistent use of decimals instead of floats":
(https://github.com/bitcoin/bitcoin/pull/31595#issuecomment-2572924040)
> Why? While we are currently relying on the `short` python `repr()` algorithm for floats, this has been stable and reliable since it's been introduced in python 3.1 as far as I know - so I don't really see the benefit of changing dozens of functional tests / adding a new linter - unless there have been other bugs related to this in the past?
Consider two lines from [`test/functional/rpc_psbt.py`](https://github.com/bitcoin/bitcoin/blob/master/test/functional/rpc_psbt.py):https://github.com/b
...
(https://github.com/bitcoin/bitcoin/pull/31595#issuecomment-2572924040)
> Why? While we are currently relying on the `short` python `repr()` algorithm for floats, this has been stable and reliable since it's been introduced in python 3.1 as far as I know - so I don't really see the benefit of changing dozens of functional tests / adding a new linter - unless there have been other bugs related to this in the past?
Consider two lines from [`test/functional/rpc_psbt.py`](https://github.com/bitcoin/bitcoin/blob/master/test/functional/rpc_psbt.py):https://github.com/b
...
🚀 fanquake merged a pull request: "doc: Install `net/py-pyzmq` port on FreeBSD for `interface_zmq.py`"
(https://github.com/bitcoin/bitcoin/pull/31526)
(https://github.com/bitcoin/bitcoin/pull/31526)
📝 fanquake converted_to_draft a pull request: "rpc, cli: add getbalances#total, and use it for -getinfo"
(https://github.com/bitcoin/bitcoin/pull/31353)
Add a "total" field in RPC getbalances to be able to easily see the total amount held in the wallet, and use that field for the wallet balances in CLI -getinfo.
Currently -getinfo only returns getbalances#mine.trusted for wallet balances. It would make sense to instead return the total balance. For instance, to see:
- watchonly balances
- reused outputs, like coins returned to a wallet from an exchange or third party service that uses a fixed (reused) address, whether `avoid_reuse` is se
...
(https://github.com/bitcoin/bitcoin/pull/31353)
Add a "total" field in RPC getbalances to be able to easily see the total amount held in the wallet, and use that field for the wallet balances in CLI -getinfo.
Currently -getinfo only returns getbalances#mine.trusted for wallet balances. It would make sense to instead return the total balance. For instance, to see:
- watchonly balances
- reused outputs, like coins returned to a wallet from an exchange or third party service that uses a fixed (reused) address, whether `avoid_reuse` is se
...
📝 l0rinc opened a pull request: "scripted-diff: Replace invalid txid in RPC examples with valid one"
(https://github.com/bitcoin/bitcoin/pull/31610)
The RPC examples previously used an invalid 63-character txid, causing errors such as:
```
build/src/bitcoin-cli gettransaction "1075db55d416d3ca199f55b6084e2115b9345e16c5cf302fc80e9d5fbf5d48d" true
error code: -8
error message:
txid must be of length 64 (not 63, for '1075db55d416d3ca199f55b6084e2115b9345e16c5cf302fc80e9d5fbf5d48d')
```
This commit replaces the invalid txid with a valid one from https://github.com/bitcoin/bitcoin/blob/df5c643f92d4a6b1ef4dfe4b9c54f902990bb54b/src/validat
...
(https://github.com/bitcoin/bitcoin/pull/31610)
The RPC examples previously used an invalid 63-character txid, causing errors such as:
```
build/src/bitcoin-cli gettransaction "1075db55d416d3ca199f55b6084e2115b9345e16c5cf302fc80e9d5fbf5d48d" true
error code: -8
error message:
txid must be of length 64 (not 63, for '1075db55d416d3ca199f55b6084e2115b9345e16c5cf302fc80e9d5fbf5d48d')
```
This commit replaces the invalid txid with a valid one from https://github.com/bitcoin/bitcoin/blob/df5c643f92d4a6b1ef4dfe4b9c54f902990bb54b/src/validat
...
💬 l0rinc commented on pull request "refactor: modernize recent `ByteType` usages and read/write functions":
(https://github.com/bitcoin/bitcoin/pull/31601#issuecomment-2572965630)
I haven't seen your commit, I've recreated it - do you want me to cherry-pick that here instead?
(https://github.com/bitcoin/bitcoin/pull/31601#issuecomment-2572965630)
I haven't seen your commit, I've recreated it - do you want me to cherry-pick that here instead?
🤔 maflcko reviewed a pull request: "fuzz: Abort if system time is called without mock time being set"
(https://github.com/bitcoin/bitcoin/pull/31549#pullrequestreview-2531996019)
I think you forgot to set it in the init function? Otherwise, the global state may be non-deterministic:
```diff
index e4e4723c74..3f3bfbeeaa 100644
--- a/src/test/fuzz/fuzz.cpp
+++ b/src/test/fuzz/fuzz.cpp
@@ -115,6 +115,7 @@ void initialize()
// - GetStrongRandBytes(), which is used for the creation of private key material.
// - Creating a BasicTestingSetup or derived class will switch to a random seed.
SeedRandomStateForTest(SeedRand::ZEROS);
+ //here
//
...
(https://github.com/bitcoin/bitcoin/pull/31549#pullrequestreview-2531996019)
I think you forgot to set it in the init function? Otherwise, the global state may be non-deterministic:
```diff
index e4e4723c74..3f3bfbeeaa 100644
--- a/src/test/fuzz/fuzz.cpp
+++ b/src/test/fuzz/fuzz.cpp
@@ -115,6 +115,7 @@ void initialize()
// - GetStrongRandBytes(), which is used for the creation of private key material.
// - Creating a BasicTestingSetup or derived class will switch to a random seed.
SeedRandomStateForTest(SeedRand::ZEROS);
+ //here
//
...
✅ kehiy closed a pull request: "doc: upgrade bitcoin core license to 2025."
(https://github.com/bitcoin/bitcoin/pull/31605)
(https://github.com/bitcoin/bitcoin/pull/31605)
💬 kehiy commented on pull request "doc: upgrade bitcoin core license to 2025.":
(https://github.com/bitcoin/bitcoin/pull/31605#issuecomment-2573001867)
@maflcko first commit was made in github and it seems i cant squash it properly. ill try to open a new pr.
(https://github.com/bitcoin/bitcoin/pull/31605#issuecomment-2573001867)
@maflcko first commit was made in github and it seems i cant squash it properly. ill try to open a new pr.