Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 sdaftuar commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#issuecomment-2453089942)
ACK 51c2394eb8158113e73659f4ae65c237813bd5d2
🤔 tdb3 reviewed a pull request: "fees: document non-monotonic estimation edge case"
(https://github.com/bitcoin/bitcoin/pull/31080#pullrequestreview-2411707754)
Approach ACK

The update suggested in https://github.com/bitcoin/bitcoin/pull/31080#discussion_r1810430953 seems to increase clarity.
💬 l0rinc commented on pull request "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`":
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2453108430)
Thanks for the hints @maflcko, I was under the impression that big-endian tests were run automatically.

### Fix

It seem that that `std::rotr` doesn't take endianness into account, so the fix basically looks like:
```C++
size_t key_rotation = 8 * key_offset;
if constexpr (std::endian::native == std::endian::big) key_rotation *= -1;
return std::rotr(key, key_rotation);
```
I've emulated the s390x behavior locally like this:

<details>
<summary>Details</summary>

```bash
brew inst
...
👍 tdb3 approved a pull request: "test: Fix intermittent issue in wallet_backwards_compatibility.py"
(https://github.com/bitcoin/bitcoin/pull/29982#pullrequestreview-2411713750)
ACK ec777917d6eba0b417dbc90b9b891240a44b7ec4

With a similar delay (150ms) induced with https://github.com/Randy808/bitcoin/commit/f39144d808975c016c1a94022a2c4251950afb7d,
Saw test failure without the `sync_mempools()` line added in this PR. After adding the line, saw consistent passing (20x runs).
🤔 hodlinator reviewed a pull request: "test: Shut down framework cleanly on RPC connection failure"
(https://github.com/bitcoin/bitcoin/pull/30660#pullrequestreview-2411714494)
Thanks for the review @tdb3!

Implemented feedback from that review, made the code a bit more Pythonic and improved comments slightly.
💬 hodlinator commented on pull request "test: Shut down framework cleanly on RPC connection failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r1826774806)
Tried using `update()`, but it resulted in things like:
```
AssertionError: [node 0] Unable to connect to bitcoind after 10s (ignored errors: {'m': 40, 'i': 120, 's': 120, 'n': 80, 'g': 40, '_': 40, 'c': 40, 'r': 40, 'e': 80, 'd': 40, 't': 40, 'a': 40, 'l': 40}, latest error: ValueError('No RPC credentials'))
```
i.e. it was iterating over the string and incrementing an entry for each unique character, which seems to match the [docs](https://docs.python.org/3/library/collections.html#collect
...
💬 hodlinator commented on pull request "test: Shut down framework cleanly on RPC connection failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r1826772195)
(Same issue with `update()`).
💬 hodlinator commented on pull request "test: Shut down framework cleanly on RPC connection failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r1826774077)
Had to add a commit (2a0c0e410e9f32dbf7af7229457889b09553c438) that refurbishes `assert_raises_message()`, but test is much cleaner now!
👋 l0rinc's pull request is ready for review: "optimization: change XOR obfuscation key from `std::vector<std::byte>(8)` to `uint64_t`"
(https://github.com/bitcoin/bitcoin/pull/31144)
💬 hodlinator commented on pull request "cmake: Add `FindQRencode` module and enable `libqrencode` package for MSVC":
(https://github.com/bitcoin/bitcoin/pull/31173#issuecomment-2453170834)
> 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 case. D
...
👍 tdb3 approved a pull request: "test: Shut down framework cleanly on RPC connection failure"
(https://github.com/bitcoin/bitcoin/pull/30660#pullrequestreview-2411750030)
Code Review re ACK 1148c4a53295e7635a813aadfde53ba5d973395c

Nice work cleaning up the existing code around the core changes. Increases readability.
💬 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