📝 JeremyRand opened a pull request: "doc: Use relative hyperlinks in release-process.md"
(https://github.com/bitcoin/bitcoin/pull/31206)
Improves usability with offline clones of the documentation.
Refs
https://github.com/bitcoin/bitcoin/pull/30025#issuecomment-2093081127
(https://github.com/bitcoin/bitcoin/pull/31206)
Improves usability with offline clones of the documentation.
Refs
https://github.com/bitcoin/bitcoin/pull/30025#issuecomment-2093081127
💬 l0rinc commented on pull request "benchmark: Improve SipHash_32b accuracy to avoid potential optimization issues":
(https://github.com/bitcoin/bitcoin/pull/30349#discussion_r1826962618)
Thanks a lot @maflcko, that reproducer was super useful!
Added a `& 0xFF` (or do we prefer `0xff`? Found about the same number of occurrences in the code) and bumped the uint8_t to get rid of the overflow warning, see: https://github.com/bitcoin/bitcoin/compare/100cded580cfe9d69cc30866c29697d9658b4ce3..42066f45ff5d48e78a317eda63c035809bd657c6
(https://github.com/bitcoin/bitcoin/pull/30349#discussion_r1826962618)
Thanks a lot @maflcko, that reproducer was super useful!
Added a `& 0xFF` (or do we prefer `0xff`? Found about the same number of occurrences in the code) and bumped the uint8_t to get rid of the overflow warning, see: https://github.com/bitcoin/bitcoin/compare/100cded580cfe9d69cc30866c29697d9658b4ce3..42066f45ff5d48e78a317eda63c035809bd657c6
💬 Veri-max commented on issue "Source code mapping for debugger has changed since cmake":
(https://github.com/bitcoin/bitcoin/issues/31204#issuecomment-2453411805)
> developer-notes.md says:
>
> > 1. Configure source file mapping.
> >
> > For `gdb` create or append to `.gdbinit` file:
> > ```
> > set substitute-path ./src /path/to/project/root/src
> > ```
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > For `lldb` create or append to `.lldbinit` file:
> > ```
> > settings set target.source-map ./src /path/to/project/root/src
> > ```
>
> But I found I needed to create a `.lldbinit` file with
...
(https://github.com/bitcoin/bitcoin/issues/31204#issuecomment-2453411805)
> developer-notes.md says:
>
> > 1. Configure source file mapping.
> >
> > For `gdb` create or append to `.gdbinit` file:
> > ```
> > set substitute-path ./src /path/to/project/root/src
> > ```
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > For `lldb` create or append to `.lldbinit` file:
> > ```
> > settings set target.source-map ./src /path/to/project/root/src
> > ```
>
> But I found I needed to create a `.lldbinit` file with
...
⚠️ JeremyRand opened an issue: "guix: Linux and macOS builds are not cross-arch reproducible with powerpc64le build arch"
(https://github.com/bitcoin/bitcoin/issues/31207)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
The Guix output hashes for Windows and the source tarball match between my powerpc64le build machine and the official binaries, but the hashes for macOS and Linux do not match.
This seems to be a partial regression from 27.1, where Windows, Linux, and the source tarball all matched for me. macOS did not even build without errors for 27.1 on powerpc64le (that is at least fixed in 28.0),
...
(https://github.com/bitcoin/bitcoin/issues/31207)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
The Guix output hashes for Windows and the source tarball match between my powerpc64le build machine and the official binaries, but the hashes for macOS and Linux do not match.
This seems to be a partial regression from 27.1, where Windows, Linux, and the source tarball all matched for me. macOS did not even build without errors for 27.1 on powerpc64le (that is at least fixed in 28.0),
...
📝 Abdulkbk converted_to_draft a pull request: "Improve lockunspent validation for vout"
(https://github.com/bitcoin/bitcoin/pull/31205)
This PR adds a check for the vout value passed during the `lockunspent` RPC call. The code verifies if a floating-point number is provided and returns a descriptive error message.
(https://github.com/bitcoin/bitcoin/pull/31205)
This PR adds a check for the vout value passed during the `lockunspent` RPC call. The code verifies if a floating-point number is provided and returns a descriptive error message.
💬 theStack commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1827125042)
in commit 6fa10b4537b03b1746fd899de20b6f10dd6e15f0: seems like the ephemeral invariants check could return too early, potentially skipping lots of mempool txs?
```suggestion
if (dust_indexes.empty()) continue;
```
(same for the no-children condition below)
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1827125042)
in commit 6fa10b4537b03b1746fd899de20b6f10dd6e15f0: seems like the ephemeral invariants check could return too early, potentially skipping lots of mempool txs?
```suggestion
if (dust_indexes.empty()) continue;
```
(same for the no-children condition below)
👋 Abdulkbk's pull request is ready for review: "Improve lockunspent validation for vout"
(https://github.com/bitcoin/bitcoin/pull/31205)
(https://github.com/bitcoin/bitcoin/pull/31205)
👍 rkrux approved a pull request: "test: enable running independent functional test sub-tests"
(https://github.com/bitcoin/bitcoin/pull/30991#pullrequestreview-2412299380)
Concept ACK cc21876b125930f8320dbb95016f9ee7c1ffec55
Successful make and functional tests. Tried running custom methods with the new `--test_methods` argument.
Left couple suggestions.
(https://github.com/bitcoin/bitcoin/pull/30991#pullrequestreview-2412299380)
Concept ACK cc21876b125930f8320dbb95016f9ee7c1ffec55
Successful make and functional tests. Tried running custom methods with the new `--test_methods` argument.
Left couple suggestions.
💬 rkrux commented on pull request "test: enable running independent functional test sub-tests":
(https://github.com/bitcoin/bitcoin/pull/30991#discussion_r1827276868)
The PR title states that this will enable running functional test sub-tests but any method can be run with this argument at the moment such as one below. Should we restrict only for those that have the `test_` prefix?
```
+ def sample_calculator(self):
+ self.log.info("Sample calculator running")
+ x = 2 + 3
+ self.log.info(f'Calculated x to be: {x}')
+
```
(https://github.com/bitcoin/bitcoin/pull/30991#discussion_r1827276868)
The PR title states that this will enable running functional test sub-tests but any method can be run with this argument at the moment such as one below. Should we restrict only for those that have the `test_` prefix?
```
+ def sample_calculator(self):
+ self.log.info("Sample calculator running")
+ x = 2 + 3
+ self.log.info(f'Calculated x to be: {x}')
+
```
💬 rkrux commented on pull request "test: enable running independent functional test sub-tests":
(https://github.com/bitcoin/bitcoin/pull/30991#discussion_r1827273054)
Since the argument is a list named `test_methods`, let's call this function `run_custom_tests`?
(https://github.com/bitcoin/bitcoin/pull/30991#discussion_r1827273054)
Since the argument is a list named `test_methods`, let's call this function `run_custom_tests`?
💬 rkrux commented on pull request "test: enable running independent functional test sub-tests":
(https://github.com/bitcoin/bitcoin/pull/30991#discussion_r1827274569)
Might as well find the searchable & callable methods initially and execute them while throwing the error for those that are filtered out? Otherwise few would be executed and the first incorrect one would throw error.
(https://github.com/bitcoin/bitcoin/pull/30991#discussion_r1827274569)
Might as well find the searchable & callable methods initially and execute them while throwing the error for those that are filtered out? Otherwise few would be executed and the first incorrect one would throw error.
💬 maflcko commented on pull request "Improve lockunspent validation for vout":
(https://github.com/bitcoin/bitcoin/pull/31205#issuecomment-2454015922)
Can you motivate this a bit better? `getInt` is used in many RPCs. What makes this call to `getInt` in this RPC special? What real-world end-user-facing issue is this trying to solve?
(https://github.com/bitcoin/bitcoin/pull/31205#issuecomment-2454015922)
Can you motivate this a bit better? `getInt` is used in many RPCs. What makes this call to `getInt` in this RPC special? What real-world end-user-facing issue is this trying to solve?
💬 maflcko commented on issue "guix: Linux and macOS builds are not cross-arch reproducible with powerpc64le build arch":
(https://github.com/bitcoin/bitcoin/issues/31207#issuecomment-2454024773)
> I would be happy to upload my binaries and/or run diffoscope on request. I would also be happy to run Guix builds of additional commits on my powerpc64le machine if there are commits that seem likely to be the culprit.
I'd say all of that would be useful. You link to the GCC bump to 12, which seems one of the more significant changes to the Linux build, so testing commit e1ce5b8ae9124717c00dca71a5c5b43a7f5ad177 (and `e1ce5b8ae9124717c00dca71a5c5b43a7f5ad177~1`), or otherwise bisecting could
...
(https://github.com/bitcoin/bitcoin/issues/31207#issuecomment-2454024773)
> I would be happy to upload my binaries and/or run diffoscope on request. I would also be happy to run Guix builds of additional commits on my powerpc64le machine if there are commits that seem likely to be the culprit.
I'd say all of that would be useful. You link to the GCC bump to 12, which seems one of the more significant changes to the Linux build, so testing commit e1ce5b8ae9124717c00dca71a5c5b43a7f5ad177 (and `e1ce5b8ae9124717c00dca71a5c5b43a7f5ad177~1`), or otherwise bisecting could
...
💬 maflcko commented on pull request "benchmark: Improve SipHash_32b accuracy to avoid potential optimization issues":
(https://github.com/bitcoin/bitcoin/pull/30349#issuecomment-2454033675)
review ACK 42066f45ff5d48e78a317eda63c035809bd657c6
Didn't re-check ubsan
(https://github.com/bitcoin/bitcoin/pull/30349#issuecomment-2454033675)
review ACK 42066f45ff5d48e78a317eda63c035809bd657c6
Didn't re-check ubsan
💬 maflcko commented on pull request "doc: Use relative hyperlinks in release-process.md":
(https://github.com/bitcoin/bitcoin/pull/31206#issuecomment-2454043965)
review ACK 9f71cff6ab32c04149924699a68f30eebf25955b
mlc also checks relative links, according to the link you provided, so might as well use relative ones, but no strong opinion.
(https://github.com/bitcoin/bitcoin/pull/31206#issuecomment-2454043965)
review ACK 9f71cff6ab32c04149924699a68f30eebf25955b
mlc also checks relative links, according to the link you provided, so might as well use relative ones, but no strong opinion.
💬 maflcko commented on pull request "test: Fix intermittent issue in wallet_backwards_compatibility.py":
(https://github.com/bitcoin/bitcoin/pull/29982#issuecomment-2454049667)
lgtm as a temporary workaround, but I think the underlying issue should still be fixed
(https://github.com/bitcoin/bitcoin/pull/29982#issuecomment-2454049667)
lgtm as a temporary workaround, but I think the underlying issue should still be fixed
💬 hodlinator commented on pull request "cmake: Add `FindQRencode` module and enable `libqrencode` package for MSVC":
(https://github.com/bitcoin/bitcoin/pull/31173#issuecomment-2454107063)
And vcpkg has a global index similar to Rust crates etc: https://vcpkg.io/en/package/libqrencode. That was the piece I was missing from the puzzle.
Thanks for bearing with me! Haven't knowingly used vcpkg for Windows dev before.
(https://github.com/bitcoin/bitcoin/pull/31173#issuecomment-2454107063)
And vcpkg has a global index similar to Rust crates etc: https://vcpkg.io/en/package/libqrencode. That was the piece I was missing from the puzzle.
Thanks for bearing with me! Haven't knowingly used vcpkg for Windows dev before.
👍 hodlinator approved a pull request: "cmake: Add `FindQRencode` module and enable `libqrencode` package for MSVC"
(https://github.com/bitcoin/bitcoin/pull/31173#pullrequestreview-2412505962)
ACK c9baf41372b06e08976a3daf459468d3f6ad28ea
Great to have more fully-fledged builds by default when building on Windows.
CMake & vcpkg novice here but can't spot anything funky going on. Guess one attack vector is modifying the global vcpkg index to make QREncode contain malicious code, but that's implied in vcpkg builds. Official Windows releases are cross-compiled from Linux without using vcpkg, so this risk should mostly affect devs.
Should probably remove this line from the docs:
...
(https://github.com/bitcoin/bitcoin/pull/31173#pullrequestreview-2412505962)
ACK c9baf41372b06e08976a3daf459468d3f6ad28ea
Great to have more fully-fledged builds by default when building on Windows.
CMake & vcpkg novice here but can't spot anything funky going on. Guess one attack vector is modifying the global vcpkg index to make QREncode contain malicious code, but that's implied in vcpkg builds. Official Windows releases are cross-compiled from Linux without using vcpkg, so this risk should mostly affect devs.
Should probably remove this line from the docs:
...
💬 josibake commented on issue "Faster way to get block with prevouts in JSON-RPC":
(https://github.com/bitcoin/bitcoin/issues/30495#issuecomment-2454144977)
I think there are two separate topics here:
1. "I need to process the entire blockchain for [an external application like electrs, data analysis, etc]"
2. We can probably make the JSON-RPC faster, via threading, batching, etc
For 1., @vostrnad have you seen #30595 ? For the specific ask of prevouts, I'm almost certain this will always be faster since the the kernel API provides the prevouts by reading the rev.dat files (admittedly, I haven't looked into how this is done with the `getbloc
...
(https://github.com/bitcoin/bitcoin/issues/30495#issuecomment-2454144977)
I think there are two separate topics here:
1. "I need to process the entire blockchain for [an external application like electrs, data analysis, etc]"
2. We can probably make the JSON-RPC faster, via threading, batching, etc
For 1., @vostrnad have you seen #30595 ? For the specific ask of prevouts, I'm almost certain this will always be faster since the the kernel API provides the prevouts by reading the rev.dat files (admittedly, I haven't looked into how this is done with the `getbloc
...
✅ maflcko closed a pull request: "test: use context managers and add file existence checks in feature_fee_estimation.py"
(https://github.com/bitcoin/bitcoin/pull/31030)
(https://github.com/bitcoin/bitcoin/pull/31030)