🤔 ismaelsadeeq reviewed a pull request: "test: report detailed msg during utf8 response decoding error"
(https://github.com/bitcoin/bitcoin/pull/31251#pullrequestreview-2521230822)
utACK a2c45ae5480a2ee643665d6ecaee9714a287a70e
(https://github.com/bitcoin/bitcoin/pull/31251#pullrequestreview-2521230822)
utACK a2c45ae5480a2ee643665d6ecaee9714a287a70e
💬 ismaelsadeeq commented on pull request "test: report detailed msg during utf8 response decoding error":
(https://github.com/bitcoin/bitcoin/pull/31251#discussion_r1896183135)
It is okay to catch the exception as `UnicodeDecodeError` here, as the type of `data` is going to be `bytes` always?
If `data` were not `bytes`, an error would also occur because `data` would not have a `.decode` method
(https://github.com/bitcoin/bitcoin/pull/31251#discussion_r1896183135)
It is okay to catch the exception as `UnicodeDecodeError` here, as the type of `data` is going to be `bytes` always?
If `data` were not `bytes`, an error would also occur because `data` would not have a `.decode` method
⚠️ tcharding opened an issue: "Stale code (that has no effect)"
(https://github.com/bitcoin/bitcoin/issues/31558)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
I am not a C++ programmer. This code looks like it has 4 lines that do nothing. I
In commit: 0fbaef9676a a call to `std::ceil` was added which makes the following `if` statement do nothing. There is no harm done but it make the code confusing for non-c++ devs to read.
```c++
CAmount CFeeRate::GetFee(uint32_t num_bytes) const
{
const int64_t nSize{num_bytes};
// Be expli
...
(https://github.com/bitcoin/bitcoin/issues/31558)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
I am not a C++ programmer. This code looks like it has 4 lines that do nothing. I
In commit: 0fbaef9676a a call to `std::ceil` was added which makes the following `if` statement do nothing. There is no harm done but it make the code confusing for non-c++ devs to read.
```c++
CAmount CFeeRate::GetFee(uint32_t num_bytes) const
{
const int64_t nSize{num_bytes};
// Be expli
...
💬 tcharding commented on issue "Stale code (that has no effect)":
(https://github.com/bitcoin/bitcoin/issues/31558#issuecomment-2560408311)
cc @achow101
(https://github.com/bitcoin/bitcoin/issues/31558#issuecomment-2560408311)
cc @achow101
🤔 tdb3 reviewed a pull request: "descriptor: remove unreachable verification for `pkh`"
(https://github.com/bitcoin/bitcoin/pull/31555#pullrequestreview-2521284624)
Approach ACK
Left a small comment, but I don't feel very strongly about it.
(https://github.com/bitcoin/bitcoin/pull/31555#pullrequestreview-2521284624)
Approach ACK
Left a small comment, but I don't feel very strongly about it.
💬 tdb3 commented on pull request "descriptor: remove unreachable verification for `pkh`":
(https://github.com/bitcoin/bitcoin/pull/31555#discussion_r1896229349)
Seems like it might be clearer and more defensive to check that we're entering with one of the appropriate contexts rather than check that we aren't entering in one of the inappropriate contexts (albeit currently the only inappropriate one).
```diff
- Assume(ctx != ParseScriptContext::P2WPKH);
+ Assume(ctx == ParseScriptContext::TOP || ctx == ParseScriptContext::P2SH || ctx == ParseScriptContext::P2WSH || ctx == ParseScriptContext::P2TR);
```
https://github.com/bitcoin/bitcoin/b
...
(https://github.com/bitcoin/bitcoin/pull/31555#discussion_r1896229349)
Seems like it might be clearer and more defensive to check that we're entering with one of the appropriate contexts rather than check that we aren't entering in one of the inappropriate contexts (albeit currently the only inappropriate one).
```diff
- Assume(ctx != ParseScriptContext::P2WPKH);
+ Assume(ctx == ParseScriptContext::TOP || ctx == ParseScriptContext::P2SH || ctx == ParseScriptContext::P2WSH || ctx == ParseScriptContext::P2TR);
```
https://github.com/bitcoin/bitcoin/b
...
⚠️ zarei673mo opened an issue: "Bitcoincore"
(https://github.com/bitcoin/bitcoin/issues/31559)
(https://github.com/bitcoin/bitcoin/issues/31559)
💬 zarei673mo commented on issue "Bitcoincore":
(https://github.com/bitcoin/bitcoin/issues/31559#issuecomment-2560477254)
https://github.com/users/zarei673mo/projects/1/views/1?filterQuery=SBW&pane=issue&itemId=9145887
(https://github.com/bitcoin/bitcoin/issues/31559#issuecomment-2560477254)
https://github.com/users/zarei673mo/projects/1/views/1?filterQuery=SBW&pane=issue&itemId=9145887
✅ pinheadmz closed an issue: "Bitcoincore"
(https://github.com/bitcoin/bitcoin/issues/31559)
(https://github.com/bitcoin/bitcoin/issues/31559)
💬 davidgumberg commented on pull request "lint: Move assertion linter into lint runner":
(https://github.com/bitcoin/bitcoin/pull/31435#issuecomment-2560532713)
crACK https://github.com/bitcoin/bitcoin/commit/e8f0e6efaf555a7d17bdb118464bd572bd5f3933
Tested manually with the following diff:
<details>
<summary>git diff</summary>
```diff
diff --git a/src/coins.cpp b/src/coins.cpp
index 24a102b0bc..000068c7e2 100644
--- a/src/coins.cpp
+++ b/src/coins.cpp
@@ -21,6 +21,7 @@ std::unique_ptr<CCoinsViewCursor> CCoinsView::Cursor() const { return nullptr; }
bool CCoinsView::HaveCoin(const COutPoint &outpoint) const
{
+ BOOST_ASSERT(1
...
(https://github.com/bitcoin/bitcoin/pull/31435#issuecomment-2560532713)
crACK https://github.com/bitcoin/bitcoin/commit/e8f0e6efaf555a7d17bdb118464bd572bd5f3933
Tested manually with the following diff:
<details>
<summary>git diff</summary>
```diff
diff --git a/src/coins.cpp b/src/coins.cpp
index 24a102b0bc..000068c7e2 100644
--- a/src/coins.cpp
+++ b/src/coins.cpp
@@ -21,6 +21,7 @@ std::unique_ptr<CCoinsViewCursor> CCoinsView::Cursor() const { return nullptr; }
bool CCoinsView::HaveCoin(const COutPoint &outpoint) const
{
+ BOOST_ASSERT(1
...
📝 theStack opened a pull request: "rpc: allow writing UTXO set to a named pipe, introduce dump_to_sqlite.sh script"
(https://github.com/bitcoin/bitcoin/pull/31560)
This PR is based on #27432 and slightly modifies the `dumptxoutset` RPC to allow writing the UTXO set dump into a [named pipe](https://askubuntu.com/a/449192), so that the output data can be consumed by another process, see #31373. Taking use of this with the utxo-to-sqlite.py tool (introduced in #27432), creating an UTXO set in SQLite3 format is possible on the fly and becomes a one-liner with a newly introduced script `dump_to_sqlite.sh`. E.g. for signet:
```
$ ./contrib/utxo-tools/dump_to_s
...
(https://github.com/bitcoin/bitcoin/pull/31560)
This PR is based on #27432 and slightly modifies the `dumptxoutset` RPC to allow writing the UTXO set dump into a [named pipe](https://askubuntu.com/a/449192), so that the output data can be consumed by another process, see #31373. Taking use of this with the utxo-to-sqlite.py tool (introduced in #27432), creating an UTXO set in SQLite3 format is possible on the fly and becomes a one-liner with a newly introduced script `dump_to_sqlite.sh`. E.g. for signet:
```
$ ./contrib/utxo-tools/dump_to_s
...
⚠️ TwistedCrafts opened an issue: "Bitcoinnode
"
(https://github.com/bitcoin-core/gui/issues/849)
Need some focus pointer
"
(https://github.com/bitcoin-core/gui/issues/849)
Need some focus pointer
👍 i-am-yuvi approved a pull request: "test: Avoid intermittent error in assert_equal(pruneheight_new, 248)"
(https://github.com/bitcoin/bitcoin/pull/31468#pullrequestreview-2521622917)
Code Review ACK fa0998f0a028161fe7264d0e81af36ffdcb43384
I was thinking of using a batch syncing process instead of one-by-one to be memory efficient, but it ended up being a lot slower. Anyway, nice work, @maflcko.
Here are the test results:
```
Temporary test directory at /var/folders/jb/wlbrz0t95vl58wzxjt75wqmh0000gn/T/test_runner_₿_🏃_20241224_132141
Remaining jobs: [feature_index_prune.py]
1/1 - feature_index_prune.py passed, Duration: 34 s
TEST
...
(https://github.com/bitcoin/bitcoin/pull/31468#pullrequestreview-2521622917)
Code Review ACK fa0998f0a028161fe7264d0e81af36ffdcb43384
I was thinking of using a batch syncing process instead of one-by-one to be memory efficient, but it ended up being a lot slower. Anyway, nice work, @maflcko.
Here are the test results:
```
Temporary test directory at /var/folders/jb/wlbrz0t95vl58wzxjt75wqmh0000gn/T/test_runner_₿_🏃_20241224_132141
Remaining jobs: [feature_index_prune.py]
1/1 - feature_index_prune.py passed, Duration: 34 s
TEST
...
💬 Sjors commented on issue "rfc: DATUM mining interface requirements":
(https://github.com/bitcoin/bitcoin/issues/31002#issuecomment-2560811173)
> Seems like it would be trivial to add this to GBT.
Agreed.
> Seems like this would be better suited to a PR on the datum_gateway repo.
My only goal here is to figure out if the Mining interface needs anything for Datum that it doesn't need for Stratum v2. Whether you actually use it is up to you of course.
(https://github.com/bitcoin/bitcoin/issues/31002#issuecomment-2560811173)
> Seems like it would be trivial to add this to GBT.
Agreed.
> Seems like this would be better suited to a PR on the datum_gateway repo.
My only goal here is to figure out if the Mining interface needs anything for Datum that it doesn't need for Stratum v2. Whether you actually use it is up to you of course.
💬 Sjors commented on pull request "rpc: Add signet_challenge field to getblockchaininfo and getmininginfo":
(https://github.com/bitcoin/bitcoin/pull/31531#discussion_r1896523539)
Indeed the general pattern seems to be that for RPC we use `argument_name` whereas for startup arguments we use `-argumentname`.
(https://github.com/bitcoin/bitcoin/pull/31531#discussion_r1896523539)
Indeed the general pattern seems to be that for RPC we use `argument_name` whereas for startup arguments we use `-argumentname`.
💬 vasild commented on pull request "test: add mocked Sock that can read/write custom data and/or CNetMessages":
(https://github.com/bitcoin/bitcoin/pull/30205#issuecomment-2560911214)
`5766bbefa9...b448b01494`: avoid moving `GetRandomNodeEvictionCandidates()` as suggested in https://github.com/bitcoin/bitcoin/pull/30205#discussion_r1858648731 and https://github.com/bitcoin/bitcoin/pull/30205#discussion_r1745496410
(https://github.com/bitcoin/bitcoin/pull/30205#issuecomment-2560911214)
`5766bbefa9...b448b01494`: avoid moving `GetRandomNodeEvictionCandidates()` as suggested in https://github.com/bitcoin/bitcoin/pull/30205#discussion_r1858648731 and https://github.com/bitcoin/bitcoin/pull/30205#discussion_r1745496410
💬 L508726 commented on something "":
(https://github.com/bitcoin/bitcoin/commit/ce6df7df9bab2405cfe7d6e382f5682cf30de476#r150696862)
Oky my bro some how it will detamine
(https://github.com/bitcoin/bitcoin/commit/ce6df7df9bab2405cfe7d6e382f5682cf30de476#r150696862)
Oky my bro some how it will detamine
💬 vasild commented on pull request "test: add mocked Sock that can read/write custom data and/or CNetMessages":
(https://github.com/bitcoin/bitcoin/pull/30205#discussion_r1896603609)
Dropped the move.
(https://github.com/bitcoin/bitcoin/pull/30205#discussion_r1896603609)
Dropped the move.
💬 vasild commented on pull request "test: add mocked Sock that can read/write custom data and/or CNetMessages":
(https://github.com/bitcoin/bitcoin/pull/30205#discussion_r1896604403)
done, also suggested in https://github.com/bitcoin/bitcoin/pull/30205#discussion_r1745496410
(https://github.com/bitcoin/bitcoin/pull/30205#discussion_r1896604403)
done, also suggested in https://github.com/bitcoin/bitcoin/pull/30205#discussion_r1745496410