Bitcoin Core Github
44 subscribers
120K links
Download Telegram
πŸ’¬ maflcko commented on pull request "guix: update time-machine":
(https://github.com/bitcoin/bitcoin/pull/28580#issuecomment-1843630668)
I am trying to compile this on riscv64, however, it still does not seem to work.

```
dpkg --print-architecture && HOSTS="arm64-apple-darwin" V=1 VERBOSE=1 MAX_JOBS=$(nproc) ./contrib/guix/guix-build
riscv64
Found macOS SDK at '/bitcoin-core/depends/SDKs/Xcode-12.2-12B45b-extracted-SDK-with-libcxx-headers', using...
Checking that we can connect to the guix-daemon...

Hint: If this hangs, you may want to try turning your guix-daemon off and on
again.

accepted connection from pi
...
⚠️ hebasto opened an issue: "The `streams_tests/xor_file` test fails on Windows"
(https://github.com/bitcoin/bitcoin/issues/29014)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

```
>.\bitcoin-26.0\bin\test_bitcoin.exe -t streams_tests/xor_file
Running 1 test case...
unknown location(0): fatal error: in "streams_tests/xor_file": std::ios_base::failure[abi:cxx11]: AutoFile::write: file handle is nullptr: iostream error
test/streams_tests.cpp(29): last checkpoint

*** 1 failure is detected in the test module "Bitcoin Core Test Suite"

```

### Expected behav
...
πŸ’¬ fanquake commented on issue "The `streams_tests/xor_file` test fails on Windows":
(https://github.com/bitcoin/bitcoin/issues/29014#issuecomment-1843638198)
> Download https://bitcoincore.org/bin/bitcoin-core-25.1/bitcoin-25.1-win64.zip

> What version of Bitcoin Core are you using?
> v26.0

Which version is it?
πŸ’¬ hebasto commented on issue "The `streams_tests/xor_file` test fails on Windows":
(https://github.com/bitcoin/bitcoin/issues/29014#issuecomment-1843642317)
> > Download [bitcoincore.org/bin/bitcoin-core-25.1/bitcoin-25.1-win64.zip](https://bitcoincore.org/bin/bitcoin-core-25.1/bitcoin-25.1-win64.zip)
>
> > What version of Bitcoin Core are you using?
> > v26.0
>
> Which version is it?

https://bitcoincore.org/bin/bitcoin-core-26.0/bitcoin-26.0-win64.zip
πŸ’¬ TheCharlatan commented on pull request "build: Introduce internal kernel library":
(https://github.com/bitcoin/bitcoin/pull/28690#issuecomment-1843661159)
Rebased 2086d1d4c3f40cce34647995ead4bff22967ffd8 -> 34970bde23dd87fc0fea5a1970880761f7184774 ([kernelInternalLib_11](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_11) -> [kernelInternalLib_12](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_12), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelInternalLib_11..kernelInternalLib_12))
* Fixed conflict with #27581
πŸ’¬ maflcko commented on pull request "build: Introduce internal kernel library":
(https://github.com/bitcoin/bitcoin/pull/28690#discussion_r1417977573)
Is this commit needed? I guess there could be issues if the same translation unit is compiled twice?

Not sure if Hex is correct to put into "crypto".

If this isn't needed, maybe leave this for a follow-up?

An alternative would be to just remove it from the util library and keep it only in consensus?
πŸ€” pablomartin4btc reviewed a pull request: "rpc,rest,zmq: faster getblock, NotifyBlock and rest_block by reading raw block"
(https://github.com/bitcoin/bitcoin/pull/26415#pullrequestreview-1768668905)
Concept ACK

I'd like to run the bench using #26684 which I have pending of reviewing.
πŸ’¬ furszy commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#discussion_r1418007227)
> My point is that for developers, we already have tracepoints as a tool for debugging and observability into low-level details of how the software is working. In many cases, a developer will try to reproduce an issue reported by a user on their own node, at which point debuggers and tracepoints are going to be more useful than logs.

Replicate issues without information is by far more challenging than replicating them with information. It is a blind guess.

Also, what you are saying works (
...
πŸ’¬ TheCharlatan commented on pull request "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly":
(https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1418034290)
Thanks for the rationale, I indeed did not think of the future IPC delay. If we only start processing events after `app.exec()`, why not move this to just before `registerShutdownBlockReason`? Looking at the history it looks to me like this was put where it is to be in the general vicinity of the `installEvent*` calls.
πŸ“ ryanofsky opened a pull request: "kernel: Streamline util library"
(https://github.com/bitcoin/bitcoin/pull/29015)
Remove `fees.h`, `errors.h`, and `spanparsing.h` from the util library. Specifically:

- Move `Split` functions from `util/spanparsing.h` to `util/string.h`, using `util` namespace for clarity.
- Move remaining spanparsing functions to `script/parsing.h` since they are used for descriptor and miniscript parsing.
- Combine `util/fees.h` and `util/errors.h` into `common/messages.h` so there is a place for simple functions that generate user messages to live, and these functions are not part of
...
πŸ‘ TheCharlatan approved a pull request: "build: use macOS 14 SDK (Xcode 15.0)"
(https://github.com/bitcoin/bitcoin/pull/28622#pullrequestreview-1768741962)
ACK 8ea45e626e5a0482ee11d4376f961d8126ce7c7b

Get the same build hashes and SDK hashes. Ran the binary on macOS 11.1. Completed a depends build on my linux dev box. Verified the boost process patch.
πŸ“ niftynei opened a pull request: "RPC: add new `listmempooltransactions`"
(https://github.com/bitcoin/bitcoin/pull/29016)
## Proposed Update

Add a new RPC endpoint, `listmempooltransactions`. Takes as args a `start_sequence` and `verbose`.

Returns:
- if verbose false, list of txids + their `entry_sequence` where each entry's `entry_sequence` >= the provided `start_sequence.
- if verbose true, raw tx output info including each entry's `entry_sequence`.


Builds on work done in #19572.

## Rationale
The current mempool RPCs are lacking an ability to scan for updates in a more efficient manner. You can
...
⚠️ centaur1 opened an issue: "Build error on Ubuntu 22.04.3 LTS"
(https://github.com/bitcoin/bitcoin/issues/29017)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Building on 22.04.3 LTS (GNU/Linux 5.15.0-87-generic x86_64) results in this error:

test/base58_tests.cpp: In member function β€˜void base58_tests::base58_EncodeBase58::test_method()’:
test/base58_tests.cpp:26:44: error: invalid initialization of reference of type β€˜const string&’ {aka β€˜const std::__cxx11::basic_string<char>&’} from expression of type β€˜const unsigned char [1462]’
26 |
...
πŸ’¬ ryanofsky commented on pull request "build: Introduce internal kernel library":
(https://github.com/bitcoin/bitcoin/pull/28690#discussion_r1418057188)
re: https://github.com/bitcoin/bitcoin/pull/28690#discussion_r1417977573

> Is this commit needed? I guess there could be issues if the same translation unit is compiled twice?
>
> Not sure if Hex is correct to put into "crypto".
>
> If this isn't needed, maybe leave this for a follow-up?
>
> An alternative would be to just remove it from the util library and keep it only in consensus?

Not sure about the current state, but when this was asked previously https://github.com/bitcoin/b
...
πŸ‘ ryanofsky approved a pull request: "build: Introduce internal kernel library"
(https://github.com/bitcoin/bitcoin/pull/28690#pullrequestreview-1768762282)
Code review ACK 34970bde23dd87fc0fea5a1970880761f7184774.

Main change since last review is no longer moving the spanparsing.h file here, to avoid making util depend on common. I have since opened a separate PR #29015 to remove spanparsing.h functions from util by splitting it up.
πŸ€” mzumsande reviewed a pull request: "test/BIP324: functional tests for v2 P2P encryption"
(https://github.com/bitcoin/bitcoin/pull/24748#pullrequestreview-1768762854)
ACK f479a99cc92c7966266804e8c33338cebbc44fbc

I have reviewed the code. I did some testing / gained more confidence that it works well by having a [branch](https://github.com/mzumsande/bitcoin/tree/tmp_bip324_fixalltests) where, with some adjustments, the python v2 p2p module is used for the entire functional test suite.
πŸ’¬ mzumsande commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1418061039)
One thing to consider is that now we have two locks for p2p, we need to worry about deadlocks.
There is an implied lock order, `on_message` locks `p2p_lock`, and can call messages (e.g. `on_inv`) that will call `send_message` and lock `_send_lock`. So we must make sure never to take `p2p_lock` after `_send_lock`. I don't think that this is currently possible, but maybe we should add a comment about this.
πŸ’¬ TheCharlatan commented on pull request "build: Introduce internal kernel library":
(https://github.com/bitcoin/bitcoin/pull/28690#discussion_r1418067302)
Re https://github.com/bitcoin/bitcoin/pull/28690#discussion_r1417977573
> An alternative would be to just remove it from the util library and keep it only in consensus?

These functions are used by components of `bitcoin-cli`. The `bitcoin-cli` binary should not have to rely on the consensus library, so I don't think that is an alternative.
πŸ’¬ ryanofsky commented on pull request "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly":
(https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1418077030)
> Thanks for the rationale, I indeed did not think of the future IPC delay. If we only start processing events after `app.exec()`, why not move this to just before `registerShutdownBlockReason`? Looking at the history it looks to me like this was put where it is to be in the general vicinity of the `installEvent*` calls.

I can make this change if you still think it's a good idea after this, but I think it's reasonable to want to keep the installEvent calls together. I also didn't want to just
...
πŸ’¬ sipsorcery commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#issuecomment-1843853088)
Maybe something in txmempool.h that the msvc compiler doesn't like?

```
D:\a\bitcoin\bitcoin\src\txmempool.h(457,60): error C2143: syntax error: missing ')' before 'public' [D:\a\bitcoin\bitcoin\build_msvc\libbitcoin_qt\libbitcoin_qt.vcxproj]
D:\a\bitcoin\bitcoin\src\txmempool.h(457,60): error C2143: syntax error: missing ';' before 'public' [D:\a\bitcoin\bitcoin\build_msvc\libbitcoin_qt\libbitcoin_qt.vcxproj]
D:\a\bitcoin\bitcoin\src\txmempool.h(457,67): error C2059: syntax error: ')' [D:
...