š¬ achow101 commented on pull request "RPC: Add reserve member function to `UniValue` and use it in `blockToJSON` function":
(https://github.com/bitcoin/bitcoin/pull/31179#issuecomment-3109997312)
ACK 5d82d92aff7c11ce17ee809c060e37f73a8a687a
(https://github.com/bitcoin/bitcoin/pull/31179#issuecomment-3109997312)
ACK 5d82d92aff7c11ce17ee809c060e37f73a8a687a
š ryanofsky approved a pull request: "refactor,test: follow-ups to multi-byte block obfuscation"
(https://github.com/bitcoin/bitcoin/pull/33039#pullrequestreview-3048910436)
Code review ACK 86e3a0a8cbd30cfee98f5b4acf4ce6d0a75a3ef0, just tweaking key write assert as suggested
(https://github.com/bitcoin/bitcoin/pull/33039#pullrequestreview-3048910436)
Code review ACK 86e3a0a8cbd30cfee98f5b4acf4ce6d0a75a3ef0, just tweaking key write assert as suggested
š¬ ryanofsky commented on pull request "refactor,test: follow-ups to multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/33039#discussion_r2226584013)
In commit "test: make `obfuscation_serialize` more thorough" (2dea0454254180d79464dc6afd3312b1caf369a7)
Could check this more strictly with:
<details><summary>diff</summary>
<p>
```diff
--- a/src/test/streams_tests.cpp
+++ b/src/test/streams_tests.cpp
@@ -65,17 +65,19 @@ BOOST_AUTO_TEST_CASE(obfuscation_serialize)
std::vector key_in{m_rng.randbytes<std::byte>(Obfuscation::KEY_SIZE)};
DataStream ds_in;
ds_in << key_in;
- BOOST_CHECK_EQUAL(ds_in.size(), 1 + Obfus
...
(https://github.com/bitcoin/bitcoin/pull/33039#discussion_r2226584013)
In commit "test: make `obfuscation_serialize` more thorough" (2dea0454254180d79464dc6afd3312b1caf369a7)
Could check this more strictly with:
<details><summary>diff</summary>
<p>
```diff
--- a/src/test/streams_tests.cpp
+++ b/src/test/streams_tests.cpp
@@ -65,17 +65,19 @@ BOOST_AUTO_TEST_CASE(obfuscation_serialize)
std::vector key_in{m_rng.randbytes<std::byte>(Obfuscation::KEY_SIZE)};
DataStream ds_in;
ds_in << key_in;
- BOOST_CHECK_EQUAL(ds_in.size(), 1 + Obfus
...
š achow101 merged a pull request: "RPC: Add reserve member function to `UniValue` and use it in `blockToJSON` function"
(https://github.com/bitcoin/bitcoin/pull/31179)
(https://github.com/bitcoin/bitcoin/pull/31179)
š¬ darosior commented on pull request "script: return verification flag responsible for error upon validation failure":
(https://github.com/bitcoin/bitcoin/pull/33012#issuecomment-3110064945)
One example where the opportunistic disconnection could be useful is in the case of a hardfork coin with signature replay protection but still connecting to Bitcoin peers. It happened in the past but admittedly seems very unlikely nowadays.
(https://github.com/bitcoin/bitcoin/pull/33012#issuecomment-3110064945)
One example where the opportunistic disconnection could be useful is in the case of a hardfork coin with signature replay protection but still connecting to Bitcoin peers. It happened in the past but admittedly seems very unlikely nowadays.
š¤ Prabhat1308 reviewed a pull request: "wallet: Set minversion to FEATURE_LATEST during migration"
(https://github.com/bitcoin/bitcoin/pull/33041#pullrequestreview-3048953524)
Agree with the comment by @pablomartin4btc here that if we want to go the route of keeping the wallet Version , its better to set it as a constant since `FEATURE_LATEST` is the only version being maintained in the practical sense and rename the function along the lines of `SetVersion` . Then we can even go ahead with the modified https://github.com/bitcoin/bitcoin/pull/32977 cleanup .
(https://github.com/bitcoin/bitcoin/pull/33041#pullrequestreview-3048953524)
Agree with the comment by @pablomartin4btc here that if we want to go the route of keeping the wallet Version , its better to set it as a constant since `FEATURE_LATEST` is the only version being maintained in the practical sense and rename the function along the lines of `SetVersion` . Then we can even go ahead with the modified https://github.com/bitcoin/bitcoin/pull/32977 cleanup .
š¬ l0rinc commented on pull request "refactor,test: follow-ups to multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/33039#discussion_r2226638379)
Thanks, I'll consider it next time I'm pushing
(https://github.com/bitcoin/bitcoin/pull/33039#discussion_r2226638379)
Thanks, I'll consider it next time I'm pushing
š¤ w0xlt reviewed a pull request: "wallet: Set minversion to FEATURE_LATEST during migration"
(https://github.com/bitcoin/bitcoin/pull/33041#pullrequestreview-3049035879)
ACK https://github.com/bitcoin/bitcoin/pull/33041/commits/5aa758861cf1399842fd0bea3a42d5b0cafcb0f6
(https://github.com/bitcoin/bitcoin/pull/33041#pullrequestreview-3049035879)
ACK https://github.com/bitcoin/bitcoin/pull/33041/commits/5aa758861cf1399842fd0bea3a42d5b0cafcb0f6
š¬ achow101 commented on pull request "test: reduce runtime of p2p_opportunistic_1p1c.py":
(https://github.com/bitcoin/bitcoin/pull/33048#issuecomment-3110151117)
The runtime is now much better, thanks!
ACK eb65f57f319dc4e2ea8c83cf7e283c36f1c0d53b
I checked that the changes looked sane, but did not exhaustively walkthrough the scenarios to determine whether evictions were occurring.
(https://github.com/bitcoin/bitcoin/pull/33048#issuecomment-3110151117)
The runtime is now much better, thanks!
ACK eb65f57f319dc4e2ea8c83cf7e283c36f1c0d53b
I checked that the changes looked sane, but did not exhaustively walkthrough the scenarios to determine whether evictions were occurring.
š¬ w0xlt commented on pull request "wallet, sqlite: Encapsulate SQLite statements in a RAII class":
(https://github.com/bitcoin/bitcoin/pull/33033#discussion_r2226679396)
Is there any specific reason not to use `unsigned char` directly, like `std::span<const unsigned char > sp)` ?
(https://github.com/bitcoin/bitcoin/pull/33033#discussion_r2226679396)
Is there any specific reason not to use `unsigned char` directly, like `std::span<const unsigned char > sp)` ?
š¬ achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2226689907)
Will change if I need to retouch.
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2226689907)
Will change if I need to retouch.
š¬ furszy commented on pull request "index: fix wrong assert of current_tip == m_best_block_index":
(https://github.com/bitcoin/bitcoin/pull/32878#issuecomment-3110184342)
@HowHsu, are you going to include the test or a patch to reproduce the issue? Iām happy to ACK it either way.
(https://github.com/bitcoin/bitcoin/pull/32878#issuecomment-3110184342)
@HowHsu, are you going to include the test or a patch to reproduce the issue? Iām happy to ACK it either way.
š¬ w0xlt commented on pull request "wallet: Store transactions in a separate sqlite table":
(https://github.com/bitcoin/bitcoin/pull/33034#issuecomment-3110194893)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/33034#issuecomment-3110194893)
Concept ACK
š¬ achow101 commented on pull request "wallet, sqlite: Encapsulate SQLite statements in a RAII class":
(https://github.com/bitcoin/bitcoin/pull/33033#discussion_r2226706401)
In theory we could change `uint256`'s `value_type`, and we wouldn't need to make any changes here in that case.
(https://github.com/bitcoin/bitcoin/pull/33033#discussion_r2226706401)
In theory we could change `uint256`'s `value_type`, and we wouldn't need to make any changes here in that case.
š¤ w0xlt reviewed a pull request: "wallet: Prepare for future upgrades by recording versions of last client to open and decrypt"
(https://github.com/bitcoin/bitcoin/pull/32895#pullrequestreview-3049112073)
ACK https://github.com/bitcoin/bitcoin/pull/32895/commits/b0470f573735a04ebc87ca5d3e00a73e3f10fee1
nit: Commit https://github.com/bitcoin/bitcoin/pull/32895/commits/3662960470d74850008b7b48b6820d02002f7a5d appears to have CI failures.
(https://github.com/bitcoin/bitcoin/pull/32895#pullrequestreview-3049112073)
ACK https://github.com/bitcoin/bitcoin/pull/32895/commits/b0470f573735a04ebc87ca5d3e00a73e3f10fee1
nit: Commit https://github.com/bitcoin/bitcoin/pull/32895/commits/3662960470d74850008b7b48b6820d02002f7a5d appears to have CI failures.
š¤ w0xlt reviewed a pull request: "wallet: Set migrated wallet name only on success"
(https://github.com/bitcoin/bitcoin/pull/32984#pullrequestreview-3049114357)
reACK https://github.com/bitcoin/bitcoin/pull/32984/commits/060695c22ae7b2b0f2a1dd1417ed1b9d5a5ab542
(https://github.com/bitcoin/bitcoin/pull/32984#pullrequestreview-3049114357)
reACK https://github.com/bitcoin/bitcoin/pull/32984/commits/060695c22ae7b2b0f2a1dd1417ed1b9d5a5ab542
š¬ achow101 commented on pull request "wallet: Prepare for future upgrades by recording versions of last client to open and decrypt":
(https://github.com/bitcoin/bitcoin/pull/32895#issuecomment-3110226273)
> nit: Commit [3662960](https://github.com/bitcoin/bitcoin/commit/3662960470d74850008b7b48b6820d02002f7a5d) appears to have CI failures.
I believe that's only because the tasks were canceled by a new push.
(https://github.com/bitcoin/bitcoin/pull/32895#issuecomment-3110226273)
> nit: Commit [3662960](https://github.com/bitcoin/bitcoin/commit/3662960470d74850008b7b48b6820d02002f7a5d) appears to have CI failures.
I believe that's only because the tasks were canceled by a new push.
š¬ Karim8090 commented on issue "guix: Zip file non-determinism when building in WSL":
(https://github.com/bitcoin/bitcoin/issues/32931#issuecomment-3110253591)
Babu8090
(https://github.com/bitcoin/bitcoin/issues/32931#issuecomment-3110253591)
Babu8090
š¤ w0xlt reviewed a pull request: "wallet: Set descriptor cache upgraded flag for migrated wallets"
(https://github.com/bitcoin/bitcoin/pull/33031#pullrequestreview-3049159589)
ACK https://github.com/bitcoin/bitcoin/pull/33031/commits/4e5faaba82b4e1f42edb3a52270ea9086c1edfa9
(https://github.com/bitcoin/bitcoin/pull/33031#pullrequestreview-3049159589)
ACK https://github.com/bitcoin/bitcoin/pull/33031/commits/4e5faaba82b4e1f42edb3a52270ea9086c1edfa9
š¤ mzumsande reviewed a pull request: "net: Fix Discover() not running when using -bind=0.0.0.0:port"
(https://github.com/bitcoin/bitcoin/pull/32757#pullrequestreview-3049008730)
Does the added test succeed for others? I tried to follow the instructions and get
```
test 2025-07-23T21:50:01.787971Z TestFramework (ERROR): Address 1.1.1.1 not found in node0's local addresses: []
test 2025-07-23T21:50:01.788044Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "XXX/bitcoin/32757_bind0000/test/functional/test_framework/test_framework.py", line 195, in main
...
(https://github.com/bitcoin/bitcoin/pull/32757#pullrequestreview-3049008730)
Does the added test succeed for others? I tried to follow the instructions and get
```
test 2025-07-23T21:50:01.787971Z TestFramework (ERROR): Address 1.1.1.1 not found in node0's local addresses: []
test 2025-07-23T21:50:01.788044Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "XXX/bitcoin/32757_bind0000/test/functional/test_framework/test_framework.py", line 195, in main
...