💬 hodlinator commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2082499849)
The ignored errors map are more of a categorization, while I intentionally wanted to include the full representation for the one latest error. I agree there is a drawback in them looking like different errors. Maybe something like this would be better?
```
AssertionError: [node 0] Unable to connect to bitcoind after 60s. Ignored errors: {'missing_credentials': 240}, latest error: 'missing_credentials'/ValueError('No RPC credentials').
```
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2082499849)
The ignored errors map are more of a categorization, while I intentionally wanted to include the full representation for the one latest error. I agree there is a drawback in them looking like different errors. Maybe something like this would be better?
```
AssertionError: [node 0] Unable to connect to bitcoind after 60s. Ignored errors: {'missing_credentials': 240}, latest error: 'missing_credentials'/ValueError('No RPC credentials').
```
💬 hodlinator commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2082543230)
One remaining `\n`, which would probably fail on Windows. Agree it is marginally more readable.
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2082543230)
One remaining `\n`, which would probably fail on Windows. Agree it is marginally more readable.
💬 hodlinator commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2081515988)
I know right? :) Was aiming for speed, not pseudo-collision-resistance.
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2081515988)
I know right? :) Was aiming for speed, not pseudo-collision-resistance.
💬 hodlinator commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2082530009)
> I think I will prefer the previous one as there is a case where the test node has start without a cookie file
If you can pinpoint a specific test that calls `stop_node()` without setting up some kind of RPC connection, let me know. Without being able to send the stop-RPC, we have no way of signalling the node to shut down AFAIK. We should be calling something like `TestNode.kill_process()` instead for such cases. So, seems like a logic error worthy of an assert.
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2082530009)
> I think I will prefer the previous one as there is a case where the test node has start without a cookie file
If you can pinpoint a specific test that calls `stop_node()` without setting up some kind of RPC connection, let me know. Without being able to send the stop-RPC, we have no way of signalling the node to shut down AFAIK. We should be calling something like `TestNode.kill_process()` instead for such cases. So, seems like a logic error worthy of an assert.
💬 hodlinator commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2081519646)
I like your final version for its simplicity and also being more correct as currently we only mutate the first found instance.
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2081519646)
I like your final version for its simplicity and also being more correct as currently we only mutate the first found instance.
💬 walkjivefly commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2867949544)
Concept NACK.
There's already more than enough garbage in the Bitcoin chain thanks to the Ordinals Taproot hijacking. There's no need to make it even easier for people to spam the chain.
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2867949544)
Concept NACK.
There's already more than enough garbage in the Bitcoin chain thanks to the Ordinals Taproot hijacking. There's no need to make it even easier for people to spam the chain.
💬 achow101 commented on pull request "miner: timelock the coinbase to the mined block's height":
(https://github.com/bitcoin/bitcoin/pull/32155#issuecomment-2867954774)
ACK a58cb3b1c12c8cb75a87375c50f94c4605bb805d
(https://github.com/bitcoin/bitcoin/pull/32155#issuecomment-2867954774)
ACK a58cb3b1c12c8cb75a87375c50f94c4605bb805d
🚀 achow101 merged a pull request: "miner: timelock the coinbase to the mined block's height"
(https://github.com/bitcoin/bitcoin/pull/32155)
(https://github.com/bitcoin/bitcoin/pull/32155)
💬 achow101 commented on pull request "qt, wallet: Convert uint256 to Txid":
(https://github.com/bitcoin/bitcoin/pull/32238#issuecomment-2868051681)
ACK 0671d66a8ee07584fab6f1ddaaa188c6a4ac25c1
> Good point. It's used in at least the `TransactionView::bumpedFee` signal, which might be broken now.
Tested that manually and it seems to work as expected. There is no change in visual behavior between this PR and master when fee bumping from the gui.
(https://github.com/bitcoin/bitcoin/pull/32238#issuecomment-2868051681)
ACK 0671d66a8ee07584fab6f1ddaaa188c6a4ac25c1
> Good point. It's used in at least the `TransactionView::bumpedFee` signal, which might be broken now.
Tested that manually and it seems to work as expected. There is no change in visual behavior between this PR and master when fee bumping from the gui.
📝 theuni opened a pull request: "thread-safety: fix annotations with REVERSE_LOCK"
(https://github.com/bitcoin/bitcoin/pull/32465)
This is one of several PRs to cleanup/modernize our threading primitives.
While replacing the old critical section locks in the mining code with a `REVERSE_LOCK`, I noticed that our thread-safety annotations weren't hooked up to it. This PR gets `REVERSE_LOCK` working properly.
Firstly it modernizes the attributes as-recommended by the [clang docs](https://clang.llvm.org/docs/ThreadSafetyAnalysis.html) (ctrl+f for `USE_LOCK_STYLE_THREAD_SAFETY_ATTRIBUTES`). There's a subtle difference betw
...
(https://github.com/bitcoin/bitcoin/pull/32465)
This is one of several PRs to cleanup/modernize our threading primitives.
While replacing the old critical section locks in the mining code with a `REVERSE_LOCK`, I noticed that our thread-safety annotations weren't hooked up to it. This PR gets `REVERSE_LOCK` working properly.
Firstly it modernizes the attributes as-recommended by the [clang docs](https://clang.llvm.org/docs/ThreadSafetyAnalysis.html) (ctrl+f for `USE_LOCK_STYLE_THREAD_SAFETY_ATTRIBUTES`). There's a subtle difference betw
...
📝 theuni opened a pull request: "threading: drop CSemaphore in favor of c++20 std::counting_semaphore"
(https://github.com/bitcoin/bitcoin/pull/32466)
This is relatively simple, but done in a bunch of commits to enable scripted diffs.
I wanted to add a semaphore in a branch I've been working on, but it was unclear if I should use `std::counting_semaphore` or stick with our old `CSemaphore`. I couldn't decide, so I just decided to remove all doubt and get rid of ours :)
This replaces our old `CSemaphore` with `std::counting_semaphore` everywhere we used it. `CSemaphoreGrant` is still there as an RAII wrapper, but is now called `CountingSe
...
(https://github.com/bitcoin/bitcoin/pull/32466)
This is relatively simple, but done in a bunch of commits to enable scripted diffs.
I wanted to add a semaphore in a branch I've been working on, but it was unclear if I should use `std::counting_semaphore` or stick with our old `CSemaphore`. I couldn't decide, so I just decided to remove all doubt and get rid of ours :)
This replaces our old `CSemaphore` with `std::counting_semaphore` everywhere we used it. `CSemaphoreGrant` is still there as an RAII wrapper, but is now called `CountingSe
...
🤔 ZKcash-IrishGov reviewed a pull request: "test: refactor: overhaul (w)txid determination for `CTransaction` objects"
(https://github.com/bitcoin/bitcoin/pull/32421#pullrequestreview-2830010473)
**[](https://github.com/solana-foundation/solana-improvement-documents/actions/workflows/ci.yml)[](https://github.com/solana-foundation/solana-improvement-documents/actions/workflows/ci.yml)**
(https://github.com/bitcoin/bitcoin/pull/32421#pullrequestreview-2830010473)
**[](https://github.com/solana-foundation/solana-improvement-documents/actions/workflows/ci.yml)[](https://github.com/solana-foundation/solana-improvement-documents/actions/workflows/ci.yml)**
📝 theuni opened a pull request: "checkqueue: make the queue non-optional for CCheckQueueControl and drop legacy locking macro usage"
(https://github.com/bitcoin/bitcoin/pull/32467)
As part of an effort to cleanup our threading primitives and add safe `SharedMutex`/`SharedLock` impls, I'd like to get rid of the last of our legacy `ENTER_CRITICAL_SECTION`/`LEAVE_CRITICAL_SECTION` usage. This, along with a follow-up [after fixing REVERSE_LOCK](https://github.com/bitcoin/bitcoin/pull/32465) will allow us to do that.
This replaces the old macros with an RAII lock, while simplifying `CCheckQueueControl`. It now requires a `CCheckQueue`, and optionality is handled externally.
...
(https://github.com/bitcoin/bitcoin/pull/32467)
As part of an effort to cleanup our threading primitives and add safe `SharedMutex`/`SharedLock` impls, I'd like to get rid of the last of our legacy `ENTER_CRITICAL_SECTION`/`LEAVE_CRITICAL_SECTION` usage. This, along with a follow-up [after fixing REVERSE_LOCK](https://github.com/bitcoin/bitcoin/pull/32465) will allow us to do that.
This replaces the old macros with an RAII lock, while simplifying `CCheckQueueControl`. It now requires a `CCheckQueue`, and optionality is handled externally.
...
💬 Bue-von-hon commented on pull request "rpc: Support v3 raw transactions creation":
(https://github.com/bitcoin/bitcoin/pull/31936#issuecomment-2868232853)
@adamandrews1 @achow101
As per the comments, we'll hand over the task to a more suitable developer.
Instead, we plan to proceed with [issue-26756](https://github.com/bitcoin/bitcoin/pull/26756) (we're a developer group based in South-Korea).
This issue involves aligning the behavior of the AvailableCoins method with the getbalance method, which seems well-suited to our expertise.
If you believe this issue isn’t appropriate for us, we’d appreciate it if you could suggest alternative issues.
...
(https://github.com/bitcoin/bitcoin/pull/31936#issuecomment-2868232853)
@adamandrews1 @achow101
As per the comments, we'll hand over the task to a more suitable developer.
Instead, we plan to proceed with [issue-26756](https://github.com/bitcoin/bitcoin/pull/26756) (we're a developer group based in South-Korea).
This issue involves aligning the behavior of the AvailableCoins method with the getbalance method, which seems well-suited to our expertise.
If you believe this issue isn’t appropriate for us, we’d appreciate it if you could suggest alternative issues.
...
💬 hMsats commented on issue "Self-compiled bitcoind 29.0 much slower than self-compiled 28.0 on my system":
(https://github.com/bitcoin/bitcoin/issues/32455#issuecomment-2868397512)
@sipa these were only the last 30 blocks. In total I let bitcoin-29.0 run for 56 blocks but the performance is so bad compared to when I restart bitcoin-28.0 that something else must be going on. Maybe the debug.log gives you more information?
(https://github.com/bitcoin/bitcoin/issues/32455#issuecomment-2868397512)
@sipa these were only the last 30 blocks. In total I let bitcoin-29.0 run for 56 blocks but the performance is so bad compared to when I restart bitcoin-28.0 that something else must be going on. Maybe the debug.log gives you more information?
💬 hMsats commented on issue "Self-compiled bitcoind 29.0 much slower than self-compiled 28.0 on my system":
(https://github.com/bitcoin/bitcoin/issues/32455#issuecomment-2868402344)
@maflcko I found time to run 29.0 with the following debug catagories:
```
debug=bench
debug=blockstorage
debug=cmpctblock
debug=tor
debug=validation
debug=rpc
debug=zmq
```
You can find it [here](https://bitcoinserver.nl/debug.log) and more system information on my [website](https://bitcoinserver.nl).
There are background callers. Besides a full node I run: Fulcrum (public electrum server), 2 CLN lightning nodes, electrum personal server. It's maybe a lot but 28.0 has no problems with it. At
...
(https://github.com/bitcoin/bitcoin/issues/32455#issuecomment-2868402344)
@maflcko I found time to run 29.0 with the following debug catagories:
```
debug=bench
debug=blockstorage
debug=cmpctblock
debug=tor
debug=validation
debug=rpc
debug=zmq
```
You can find it [here](https://bitcoinserver.nl/debug.log) and more system information on my [website](https://bitcoinserver.nl).
There are background callers. Besides a full node I run: Fulcrum (public electrum server), 2 CLN lightning nodes, electrum personal server. It's maybe a lot but 28.0 has no problems with it. At
...
💬 0xB10C commented on pull request "tracing: fix invalid argument in mempool_monitor":
(https://github.com/bitcoin/bitcoin/pull/32454#issuecomment-2868609868)
This came to my mind again: We might want to consider maintaining these tools outside of the Bitcoin Core repo as mentioned in #31274 6)
(https://github.com/bitcoin/bitcoin/pull/32454#issuecomment-2868609868)
This came to my mind again: We might want to consider maintaining these tools outside of the Bitcoin Core repo as mentioned in #31274 6)
💬 hebasto commented on pull request "thread-safety: fix annotations with REVERSE_LOCK":
(https://github.com/bitcoin/bitcoin/pull/32465#issuecomment-2868610302)
Concept ACK.
(https://github.com/bitcoin/bitcoin/pull/32465#issuecomment-2868610302)
Concept ACK.
💬 aryaethn commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2868616096)
Concept ACK.
Even though I am not a Bitcoin Developer, I see this of benefit for the ecosystem around Bitcoin, specially DeFi ecosystem, like Bridges, DEXs, and Lending. Removing data limit on OP_RETURN, or at least loosening the limit, provides a way for on-chain communication for DeFi dApps that try to include Bitcoin in DeFi.
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2868616096)
Concept ACK.
Even though I am not a Bitcoin Developer, I see this of benefit for the ecosystem around Bitcoin, specially DeFi ecosystem, like Bridges, DEXs, and Lending. Removing data limit on OP_RETURN, or at least loosening the limit, provides a way for on-chain communication for DeFi dApps that try to include Bitcoin in DeFi.
💬 aryaethn commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2868619847)
> Concept NACK.
>
> There's already more than enough garbage in the Bitcoin chain thanks to the Ordinals Taproot hijacking. There's no need to make it even easier for people to spam the chain.
As you know, people pay for every byte of data they post. So how is that spamming when you pay for the so-called "garbage"?
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2868619847)
> Concept NACK.
>
> There's already more than enough garbage in the Bitcoin chain thanks to the Ordinals Taproot hijacking. There's no need to make it even easier for people to spam the chain.
As you know, people pay for every byte of data they post. So how is that spamming when you pay for the so-called "garbage"?