💬 tdb3 commented on pull request "test: Shut down framework cleanly on RPC connection failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r1826867112)
non-blocking pico-nit:
If this file gets touched again, could use `f` string instead of `format`.
https://github.com/bitcoin/bitcoin/blob/master/test/functional/README.md#style-guidelines

💬 hodlinator commented on pull request "test: Shut down framework cleanly on RPC connection failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r1826869399)
Good point! Resolved in latest push.

👍 tdb3 approved a pull request: "test: Shut down framework cleanly on RPC connection failure"
(https://github.com/bitcoin/bitcoin/pull/30660#pullrequestreview-2411752016)
re ACK 042cee7073b338d79b5176d4157cf08bbd079b9f

👍 jasonribble approved a pull request: "Refactor BnB tests"
(https://github.com/bitcoin/bitcoin/pull/29532#pullrequestreview-2411753208)
ACK

I went through each of the commits. It is an improvement of the branch and bound selection tests.

🤔 JoeyVee8666 reviewed a pull request: "test: added test to assert TX decode rpc error on submitpackage rpc"
(https://github.com/bitcoin/bitcoin/pull/31139#pullrequestreview-2411758402)
Looks kinda sus...

💬 JoeyVee8666 commented on pull request "fuzz: fix `implicit-integer-sign-change` in wallet_create_transaction":
(https://github.com/bitcoin/bitcoin/pull/31203#issuecomment-2453235424)
> <!--e57a25ab6845829454e8d69fc972939a-->
>
> The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
>
> <!--006a51241073e994b41acfe9ec718e94-->
> ### Code Coverage & Benchmarks
> For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31203.
> <!--021abf342d371248e50ceaed478a90ca-->
> ### Reviews
> See [the guideline](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#code-review) for information on the review process.
> | T
...

💬 hebasto commented on pull request "cmake: Add `FindQRencode` module and enable `libqrencode` package for MSVC":
(https://github.com/bitcoin/bitcoin/pull/31173#issuecomment-2453345360)
> > The [toolchain file provided by `vcpkg`](https://github.com/bitcoin/bitcoin/blob/f1bcf3edc5027b501616670db33d8be1f2cb5a11/CMakePresets.json#L32) sets the [`CMAKE_FIND_ROOT_PATH`](https://cmake.org/cmake/help/latest/variable/CMAKE_FIND_ROOT_PATH.html) variable, directing the `find_path()` and `find_library()` commands to the appropriate paths.
>
> Okay, the toolchain file seems to be _C:\Program Files\Microsoft Visual Studio\2022\Community\VC\vcpkg\scripts\buildsystems\vcpkg.cmake_ in my c
...

📝 Abdulkbk opened 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.

📝 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

💬 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

💬 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
...

⚠️ 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),
...

📝 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.

💬 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)

👋 Abdulkbk's pull request is ready for review: "Improve lockunspent validation for vout"
(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.

💬 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}')
+
```

💬 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`?

💬 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.

💬 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?