Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 dooglus commented on issue "New crash in v26.0":
(https://github.com/bitcoin-core/gui/issues/785#issuecomment-1923669190)
It crashed again. This crash looks the same as the last one:

```
[Thread 0x7fffea7fb6c0 (LWP 2748) exited]
[Thread 0x7fffeaffc6c0 (LWP 2747) exited]

Thread 1 "bitcoin-qt-v26." received signal SIGSEGV, Segmentation fault.
QSortFilterProxyModelPrivate::build_source_to_proxy_mapping (
proxy_to_source=..., source_to_proxy=..., this=this@entry=0x555557a7d520)
at ../../include/QtCore/../../src/corelib/tools/qarraydata.h:61
61 ../../include/QtCore/../../src/corelib/tools/qarray
...
💬 theStack commented on pull request "rpc: parse legacy pubkeys consistently with specific error messages":
(https://github.com/bitcoin/bitcoin/pull/28336#discussion_r1475987895)
Sorry for the late reply. I think the exact order of the checks doesn't really matter here from a performance perspective, since in the typical case (if the passed pubkey is valid) all three of them are executed anyway. Also, if users pass in something that has a different format like e.g. a Bitcoin address, the error message "pubkey has to be in hex" seems to be more useful than "pubkey has the wrong length".
💬 delta1 commented on pull request "refactor: Allow CScript construction from any std::input_iterator":
(https://github.com/bitcoin/bitcoin/pull/29369#issuecomment-1923691732)
Concept ACK dddd56c6eecfb0e0f53874355ee821f5bd6af556

Looks like a nice code change to me. "previous releases" CI is turning up an issue in script/interpreter

```
script/interpreter.cpp: In function ‘bool EvalChecksigPreTapscript(const valtype&, const valtype&, prevector<28, unsigned char>::const_iterator, prevector<28, unsigned char>::const_iterator, unsigned int, const BaseSignatureChecker&, SigVersion, ScriptError*, bool&)’:
script/interpreter.cpp:325:44: error: no matching function f
...
💬 delta1 commented on pull request "refactor: Allow CScript construction from any std::input_iterator":
(https://github.com/bitcoin/bitcoin/pull/29369#issuecomment-1923698793)
> CI is turning up an issue in script/interpreter

looks like a few issues in that file
💬 maflcko commented on pull request "refactor: Fix timedata includes":
(https://github.com/bitcoin/bitcoin/pull/29361#discussion_r1475998235)
Going to leave removal of further includes to a follow-up
💬 fanquake commented on pull request "test: Drop `x` modifier in `fsbridge::fopen` call for MinGW builds":
(https://github.com/bitcoin/bitcoin/pull/29357#discussion_r1476007817)
Yea. So I don't see why anything GCC related is required, or needs to be mentioned here.
🤔 BrandonOdiwuor reviewed a pull request: "test: Assumeutxo with more than just coinbase transactions"
(https://github.com/bitcoin/bitcoin/pull/29354#pullrequestreview-1859156221)
Concept ACK
💬 theStack commented on pull request "test: use v2 everywhere for P2PConnection if --v2transport is enabled":
(https://github.com/bitcoin/bitcoin/pull/29358#issuecomment-1923734576)
Concept ACK
💬 brunoerg commented on pull request "Fuzz: extend CConnman tests":
(https://github.com/bitcoin/bitcoin/pull/28584#discussion_r1476030643)
In b1b74a13adb8e943d90fbe56c4a706b0ae91335a: nit: I don't think to set network active here is a must, we could fuzz it with both network active and inactive.
💬 maflcko commented on pull request "refactor: Allow CScript construction from any std::input_iterator":
(https://github.com/bitcoin/bitcoin/pull/29369#issuecomment-1923782707)
@delta1 This is an issue in the C++ standard library. Basically, the standard assumes that `element_type` and `value_type` types are never provided by an iterator type at the same time. This was fixed in https://cplusplus.github.io/LWG/issue3446 and https://github.com/gcc-mirror/gcc/commit/186aa6304570e15065f31482e9c27326a3a6679f
💬 BrandonOdiwuor commented on issue "test: RPC coverage check doesn't work?":
(https://github.com/bitcoin/bitcoin/issues/27593#issuecomment-1923784437)
I have reviewed the issue and it's evident that the wallet RPCs are indeed absent in the generated `rpc_interface.txt` file. This observation aligns with the analysis provided by @kouloumos.

The root cause lies in the invocation of `start_node(...)` within `create_cache.py`, where the `write_all_rpc_commands(...)` function is called. This occurs during the creation of the cache on a node that lacks wallet functionality. Consequently, the wallet RPCs are not included in the generated `rpc_int
...
💬 brunoerg commented on pull request "Fuzz: extend CConnman tests":
(https://github.com/bitcoin/bitcoin/pull/28584#discussion_r1476052411)
In d923b86264df93ad86c7bd557b9970e156585dc7: You could use `ConsumeService` into `ConsumeServiceVector`.

```diff
diff --git a/src/test/fuzz/util/net.h b/src/test/fuzz/util/net.h
index a97017555d..78e61b51d9 100644
--- a/src/test/fuzz/util/net.h
+++ b/src/test/fuzz/util/net.h
@@ -102,8 +102,7 @@ inline std::vector<CService> ConsumeServiceVector(FuzzedDataProvider& fuzzed_dat
const size_t size = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, max_vector_size);
ret.reserv
...
💬 BrandonOdiwuor commented on issue "test: RPC coverage check doesn't work?":
(https://github.com/bitcoin/bitcoin/issues/27593#issuecomment-1923793691)
@kouloumos what do you mean by?
> we can avoid this issue, but just a reordering of cache-creation<>coverage-initialization pushes the issue to the next coverage-initialization which in that case will be the first test of the test-runner. If the node in that first test doesn't have a wallet, the issue remains.

Doesn't the coverage-initialization happen inside cache-creation. Do you mean there should be a coverage-initialization (generation of the rpc_interface.txt) before `create_cache.py`
...
💬 delta1 commented on pull request "refactor: Allow CScript construction from any std::input_iterator":
(https://github.com/bitcoin/bitcoin/pull/29369#issuecomment-1923798641)
@maflcko interesting, thanks for the context. does that mean that the previous releases CI job needs to be changed to user a newer gcc?
💬 maflcko commented on issue "test: RPC coverage check doesn't work?":
(https://github.com/bitcoin/bitcoin/issues/27593#issuecomment-1923811199)
Why not simply enable the wallet in `create_cache`?
💬 josibake commented on pull request "wallet: cache IsMine scriptPubKeys to improve performance of descriptor wallets":
(https://github.com/bitcoin/bitcoin/pull/26008#issuecomment-1923826205)
> Dropped the signing changes as they were causing issues with being able to sign transactions that we have the keys for but not the descriptors. https://github.com/bitcoin/bitcoin/pull/23417 would allow us to resolve the performance issues for signing while retaining this behavior.

Is this worth revisiting since your re-write of the cache? The signing stuff could be a follow-up PR, but if it's simple enough it would be really nice to include it here. Otherwise, someone using migrate wallet c
...
💬 glozow commented on pull request "Weaken serfloat tests":
(https://github.com/bitcoin/bitcoin/pull/29192#issuecomment-1923857100)
ACK ea60c95be36a68ba98d1d23018587aa4d4d6bb1a, based on https://github.com/Homebrew/homebrew-core/pull/156745 and https://github.com/bitcoin/bitcoin/pull/29192#issuecomment-1881144857 it looks like this fixes the issue. I think this is the only thing we're waiting on for 26.1.

re https://github.com/bitcoin/bitcoin/pull/29192#issuecomment-1906925728, the values that were failing the tests are irrelevant in the context we're using this, and 21.0 has been eol for a while now - also see https://g
...
💬 epiccurious commented on pull request "test: use v2 everywhere for P2PConnection if --v2transport is enabled":
(https://github.com/bitcoin/bitcoin/pull/29358#issuecomment-1923932584)
Tested ACK.

>not actual tests that can be executed directly
Thanks for the clarification.

Certain tests take longer to run, to be expected with the added sub-tests.
```
TEST | STATUS | DURATION

feature_block.py | ✓ Passed | 197 s
feature_maxuploadtarget.py | ✓ Passed | 59 s
p2p_ibd_stalling.py | ✓ Passed | 59 s
p2p_ibd_stalling.py --v2transport | ✓ Passed | 58 s
p2p_invalid_messages.py | ✓ Passed
...
💬 stickies-v commented on pull request "[WIP] C++20 std::views::reverse":
(https://github.com/bitcoin/bitcoin/pull/28687#issuecomment-1923969918)
Force pushed to rebase, added a commit to bump compilers of failing CI tasks (see [OP](https://github.com/bitcoin/bitcoin/pull/28687#issue-1952153535)), not too sure what I'm doing here though.
🤔 murchandamus reviewed a pull request: "wallet: Set descriptors flag after migrating blank wallets"
(https://github.com/bitcoin/bitcoin/pull/29367#pullrequestreview-1859460255)
Concept ACK