💬 maflcko commented on pull request "[WIP] build: remove need to test for endianness":
(https://github.com/bitcoin/bitcoin/pull/29852#issuecomment-2604406887)
> > > MSVC
> >
> >
> > Has someone reported the request to them? If not, it seems less likely they'll fix it.
>
> https://developercommunity.visualstudio.com/t/Missed-optimization-for-consecutive-byte/10831400
Thx. Could also add a link (https://github.com/google/leveldb/commit/201f52201f5dd9701e7a8ceaa0ec4d344e69e022) to the thread to give one example that the code is used in the real world by a real software project?
(https://github.com/bitcoin/bitcoin/pull/29852#issuecomment-2604406887)
> > > MSVC
> >
> >
> > Has someone reported the request to them? If not, it seems less likely they'll fix it.
>
> https://developercommunity.visualstudio.com/t/Missed-optimization-for-consecutive-byte/10831400
Thx. Could also add a link (https://github.com/google/leveldb/commit/201f52201f5dd9701e7a8ceaa0ec4d344e69e022) to the thread to give one example that the code is used in the real world by a real software project?
💬 0xB10C commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1923521368)
Thanks! Changed the declaration from `uint64_t` to `void*`.
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1923521368)
Thanks! Changed the declaration from `uint64_t` to `void*`.
💬 0xB10C commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#issuecomment-2604412931)
Rebased, addressed https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1905788722 and fixed the linter warning.
(https://github.com/bitcoin/bitcoin/pull/25832#issuecomment-2604412931)
Rebased, addressed https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1905788722 and fixed the linter warning.
💬 maflcko commented on pull request "Added rescan option for import descriptors":
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r1923524934)
How does this interact with `timestamp`? As mentioned in the issue thread, I agree it would be better to have another special timestamp value, no?
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r1923524934)
How does this interact with `timestamp`? As mentioned in the issue thread, I agree it would be better to have another special timestamp value, no?
⚠️ fanquake opened an issue: "ci: tidy task doesn't run on aarch64"
(https://github.com/bitcoin/bitcoin/issues/31697)
```bash
time env -i HOME="$HOME" PATH="$PATH" USER="$USER" bash -c 'FILE_ENV="./ci/test/00_setup_env_native_tidy.sh" ./ci/test_run_all.sh'
<snip>
[ 0%] Built target bitcoin_crypto_arm_shani
[100%] Built target bitcoin_crypto
gmake: *** No rule to make target 'bitcoin_crypto_x86_shani'. Stop.
```
The script as written assumes that it will be run on `x86_64`:
https://github.com/bitcoin/bitcoin/blob/d7f56cc5d9e12ad31dd1ce8b34c3ff4ec5c1b70c/contrib/devtools/check-deps.sh#L11
(https://github.com/bitcoin/bitcoin/issues/31697)
```bash
time env -i HOME="$HOME" PATH="$PATH" USER="$USER" bash -c 'FILE_ENV="./ci/test/00_setup_env_native_tidy.sh" ./ci/test_run_all.sh'
<snip>
[ 0%] Built target bitcoin_crypto_arm_shani
[100%] Built target bitcoin_crypto
gmake: *** No rule to make target 'bitcoin_crypto_x86_shani'. Stop.
```
The script as written assumes that it will be run on `x86_64`:
https://github.com/bitcoin/bitcoin/blob/d7f56cc5d9e12ad31dd1ce8b34c3ff4ec5c1b70c/contrib/devtools/check-deps.sh#L11
💬 hebasto commented on pull request "qa: Fix `wallet_multiwallet.py`":
(https://github.com/bitcoin/bitcoin/pull/31410#discussion_r1923543876)
I don't think these changes can be meaningfully separated. Applying either change independently would break the test or render it incomplete.
(https://github.com/bitcoin/bitcoin/pull/31410#discussion_r1923543876)
I don't think these changes can be meaningfully separated. Applying either change independently would break the test or render it incomplete.
💬 maflcko commented on pull request "qa: Fix `wallet_multiwallet.py`":
(https://github.com/bitcoin/bitcoin/pull/31410#issuecomment-2604458007)
Only change is the commit msg
re-ACK e717fb5a429d9d00e008fa1eb2b85179050be8fe 🍣
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted com
...
(https://github.com/bitcoin/bitcoin/pull/31410#issuecomment-2604458007)
Only change is the commit msg
re-ACK e717fb5a429d9d00e008fa1eb2b85179050be8fe 🍣
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted com
...
💬 hebasto commented on pull request "qa: Fix `wallet_multiwallet.py`":
(https://github.com/bitcoin/bitcoin/pull/31410#discussion_r1923550914)
> Shouldn't we atleast test the known behaviour for our release build...?
As documented in the code comment:
> A cross-compiled bitcoind.exe parses self_walletdat_symlink without errors.
Therefore, the release build won't trigger the "Error scanning" message, and this test would fail on the master branch.
(https://github.com/bitcoin/bitcoin/pull/31410#discussion_r1923550914)
> Shouldn't we atleast test the known behaviour for our release build...?
As documented in the code comment:
> A cross-compiled bitcoind.exe parses self_walletdat_symlink without errors.
Therefore, the release build won't trigger the "Error scanning" message, and this test would fail on the master branch.
💬 fanquake commented on pull request "qa: Fix `wallet_multiwallet.py`":
(https://github.com/bitcoin/bitcoin/pull/31410#discussion_r1923551284)
> I guess the check could be limited to exclude the msvc build?
That would make the most sense to me.
(https://github.com/bitcoin/bitcoin/pull/31410#discussion_r1923551284)
> I guess the check could be limited to exclude the msvc build?
That would make the most sense to me.
💬 maflcko commented on pull request "kernel: Move block tree db open to block manager":
(https://github.com/bitcoin/bitcoin/pull/30965#issuecomment-2604463419)
Only changes are the ones mentioned already.
Also, some moved code wasn't covered by tests, so I went ahead and added them in https://github.com/bitcoin/bitcoin/pull/31696.
re-ACK 0cdddeb2240d1f33c8b2dd28bb0c9d84d9420e3d 🏪
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_w
...
(https://github.com/bitcoin/bitcoin/pull/30965#issuecomment-2604463419)
Only changes are the ones mentioned already.
Also, some moved code wasn't covered by tests, so I went ahead and added them in https://github.com/bitcoin/bitcoin/pull/31696.
re-ACK 0cdddeb2240d1f33c8b2dd28bb0c9d84d9420e3d 🏪
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_w
...
💬 fanquake commented on pull request "qa: Fix `wallet_multiwallet.py`":
(https://github.com/bitcoin/bitcoin/pull/31410#discussion_r1923553436)
> Therefore, the release build won't trigger the "Error scanning" message, and this test would fail on the master branch.
Sure, so you can check that that is happening when testing on Windows.
(https://github.com/bitcoin/bitcoin/pull/31410#discussion_r1923553436)
> Therefore, the release build won't trigger the "Error scanning" message, and this test would fail on the master branch.
Sure, so you can check that that is happening when testing on Windows.
⚠️ fanquake opened an issue: "contrib: Autoconf fragments left in test-*-check scripts"
(https://github.com/bitcoin/bitcoin/issues/31698)
The [`test-security-check`](https://github.com/bitcoin/bitcoin/blob/master/contrib/devtools/test-security-check.py) and [`test-symbol-check`](https://github.com/bitcoin/bitcoin/blob/master/contrib/devtools/test-symbol-check.py) scripts still reference Autoconf and it's behaviour:
```python
def env_flags() -> list[str]:
# This should behave the same as AC_TRY_LINK, so arrange well-known flags
# in the same order as autoconf would.
#
# See the definitions for ac_link in autoconf's
...
(https://github.com/bitcoin/bitcoin/issues/31698)
The [`test-security-check`](https://github.com/bitcoin/bitcoin/blob/master/contrib/devtools/test-security-check.py) and [`test-symbol-check`](https://github.com/bitcoin/bitcoin/blob/master/contrib/devtools/test-symbol-check.py) scripts still reference Autoconf and it's behaviour:
```python
def env_flags() -> list[str]:
# This should behave the same as AC_TRY_LINK, so arrange well-known flags
# in the same order as autoconf would.
#
# See the definitions for ac_link in autoconf's
...
💬 TheCharlatan commented on pull request "test: Check that reindex with prune wipes blk files":
(https://github.com/bitcoin/bitcoin/pull/31696#discussion_r1923573442)
Might it be good to check the file count here before deletion during the reindex?
(https://github.com/bitcoin/bitcoin/pull/31696#discussion_r1923573442)
Might it be good to check the file count here before deletion during the reindex?
🤔 TheCharlatan reviewed a pull request: "guix: use GCC 13 to build releases"
(https://github.com/bitcoin/bitcoin/pull/29881#pullrequestreview-2564364832)
Guix build (aarch64):
```
b381ff87a34e45f693abee5554dd0680cb7879d66787f8aba45fb9dd149e8e78 guix-build-0c1b29a05777/output/aarch64-linux-gnu/SHA256SUMS.part
d3cb534b26ee5edd0c5d459cfffd3ebb46235da21ce2c614fda92d56f4b6ba98 guix-build-0c1b29a05777/output/aarch64-linux-gnu/bitcoin-0c1b29a05777-aarch64-linux-gnu-debug.tar.gz
b6b73fe6c3ae291e4c3d702a1343e44f3c143b50a566f1a0412dbfcf45fe31e2 guix-build-0c1b29a05777/output/aarch64-linux-gnu/bitcoin-0c1b29a05777-aarch64-linux-gnu.tar.gz
1aa98bef1d
...
(https://github.com/bitcoin/bitcoin/pull/29881#pullrequestreview-2564364832)
Guix build (aarch64):
```
b381ff87a34e45f693abee5554dd0680cb7879d66787f8aba45fb9dd149e8e78 guix-build-0c1b29a05777/output/aarch64-linux-gnu/SHA256SUMS.part
d3cb534b26ee5edd0c5d459cfffd3ebb46235da21ce2c614fda92d56f4b6ba98 guix-build-0c1b29a05777/output/aarch64-linux-gnu/bitcoin-0c1b29a05777-aarch64-linux-gnu-debug.tar.gz
b6b73fe6c3ae291e4c3d702a1343e44f3c143b50a566f1a0412dbfcf45fe31e2 guix-build-0c1b29a05777/output/aarch64-linux-gnu/bitcoin-0c1b29a05777-aarch64-linux-gnu.tar.gz
1aa98bef1d
...
💬 saikiran57 commented on pull request "Added rescan option for import descriptors":
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r1923589979)
Hi @furszy I have updated the commit can you please review it once.
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r1923589979)
Hi @furszy I have updated the commit can you please review it once.
💬 hebasto commented on pull request "[WIP] build: remove need to test for endianness":
(https://github.com/bitcoin/bitcoin/pull/29852#issuecomment-2604542019)
> > > > MSVC
> > >
> > >
> > > Has someone reported the request to them? If not, it seems less likely they'll fix it.
> >
> >
> > https://developercommunity.visualstudio.com/t/Missed-optimization-for-consecutive-byte/10831400
>
> Thx. Could also add a link ([google/leveldb@201f522](https://github.com/google/leveldb/commit/201f52201f5dd9701e7a8ceaa0ec4d344e69e022)) to the thread to give one example that the code is used in the real world by a real software project?
https://develop
...
(https://github.com/bitcoin/bitcoin/pull/29852#issuecomment-2604542019)
> > > > MSVC
> > >
> > >
> > > Has someone reported the request to them? If not, it seems less likely they'll fix it.
> >
> >
> > https://developercommunity.visualstudio.com/t/Missed-optimization-for-consecutive-byte/10831400
>
> Thx. Could also add a link ([google/leveldb@201f522](https://github.com/google/leveldb/commit/201f52201f5dd9701e7a8ceaa0ec4d344e69e022)) to the thread to give one example that the code is used in the real world by a real software project?
https://develop
...
💬 l0rinc commented on pull request "Update leveldb subtree to latest upstream":
(https://github.com/bitcoin/bitcoin/pull/31671#issuecomment-2604566813)
Reapplying the original leveldb pulls and comparing it against this PR leaves:
```diff
diff --git a/cmake/leveldb.cmake b/cmake/leveldb.cmake
--- a/cmake/leveldb.cmake (revision 846a4dfc107d6593d0133bb8ae63edd824f7cb74)
+++ b/cmake/leveldb.cmake (date 1737461205681)
@@ -60,6 +60,7 @@
HAVE_FULLFSYNC=$<BOOL:${HAVE_FULLFSYNC}>
HAVE_O_CLOEXEC=$<BOOL:${HAVE_O_CLOEXEC}>
FALLTHROUGH_INTENDED=[[fallthrough]]
+ LEVELDB_IS_BIG_ENDIAN=$<STREQUAL:${CMAKE_CXX_BYTE_ORDER},BIG_ENDIAN
...
(https://github.com/bitcoin/bitcoin/pull/31671#issuecomment-2604566813)
Reapplying the original leveldb pulls and comparing it against this PR leaves:
```diff
diff --git a/cmake/leveldb.cmake b/cmake/leveldb.cmake
--- a/cmake/leveldb.cmake (revision 846a4dfc107d6593d0133bb8ae63edd824f7cb74)
+++ b/cmake/leveldb.cmake (date 1737461205681)
@@ -60,6 +60,7 @@
HAVE_FULLFSYNC=$<BOOL:${HAVE_FULLFSYNC}>
HAVE_O_CLOEXEC=$<BOOL:${HAVE_O_CLOEXEC}>
FALLTHROUGH_INTENDED=[[fallthrough]]
+ LEVELDB_IS_BIG_ENDIAN=$<STREQUAL:${CMAKE_CXX_BYTE_ORDER},BIG_ENDIAN
...
💬 saikiran57 commented on pull request "Added rescan option for import descriptors":
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r1923622556)
I've seen two major issues while we are importing 2M address to bitcoin using this importdescriptor method.
First issue was rescan because we are passing timestamp as 'now' so when we pass now it is taking median time of last scanned block, whereas if we take currentimestamp or futuretimestamp it is considering last mined block timestamp. So when the request is imported successfully it is setting rescan flag as true.
---- Rescan operation
1. In rescan operation it is taking either last
...
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r1923622556)
I've seen two major issues while we are importing 2M address to bitcoin using this importdescriptor method.
First issue was rescan because we are passing timestamp as 'now' so when we pass now it is taking median time of last scanned block, whereas if we take currentimestamp or futuretimestamp it is considering last mined block timestamp. So when the request is imported successfully it is setting rescan flag as true.
---- Rescan operation
1. In rescan operation it is taking either last
...
💬 krishpranav commented on issue "RFC: support for writing UTXO set dump (`dumptxoutset` RPC) to a named pipe":
(https://github.com/bitcoin/bitcoin/issues/31373#issuecomment-2604594471)
Hi, is it still open? i am looking to work on some issue.
(https://github.com/bitcoin/bitcoin/issues/31373#issuecomment-2604594471)
Hi, is it still open? i am looking to work on some issue.
💬 laanwj commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#issuecomment-2604594854)
Both CI errors are related to mempool synchronization. It seems very unlikely to be caused by the changes in this PR.
(https://github.com/bitcoin/bitcoin/pull/25832#issuecomment-2604594854)
Both CI errors are related to mempool synchronization. It seems very unlikely to be caused by the changes in this PR.