Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 ryanofsky commented on pull request "refactor: Replace g_genesis_wait_cv with m_tip_block_cv":
(https://github.com/bitcoin/bitcoin/pull/30967#discussion_r1775515667)
re: https://github.com/bitcoin/bitcoin/pull/30967#discussion_r1775163810

I think Cory's suggestion could be implemented here with:

```diff
--- a/src/node/interfaces.cpp
+++ b/src/node/interfaces.cpp
@@ -940,17 +940,11 @@ public:

BlockRef waitTipChanged(uint256 current_tip, MillisecondsDouble timeout) override
{
- // Interrupt check interval
- const MillisecondsDouble tick{1000};
- auto now{std::chrono::steady_clock::now()};
- const auto dea
...
📝 theuni opened a pull request: "build: Add missing USDT header dependency to kernel"
(https://github.com/bitcoin/bitcoin/pull/30970)
Noticed while testing a branch that replaces `boost::multi_index` with a custom replacement.

Currently depends builds pick up usdt and boost from the same path, and because boost always exists, the usdt path is implicitly included. So without boost, USDT isn't found.

An alternative to this would be to disable USDT for the kernel. I'd be open to either approach.
💬 theuni commented on pull request "build: Add missing USDT header dependency to kernel":
(https://github.com/bitcoin/bitcoin/pull/30970#issuecomment-2374557476)
Ping @hebasto @TheCharlatan
💬 ryanofsky commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#issuecomment-2374559335)
@Sjors did you re-ack the right commit hash in https://github.com/bitcoin/bitcoin/pull/30510#issuecomment-2373443353? (ACK is a for a commit that was replaced about 12 hours before the comment)
🤔 theuni reviewed a pull request: "init: Remove retry for loop"
(https://github.com/bitcoin/bitcoin/pull/30968#pullrequestreview-2328825567)
Concept ACK. I've also spent a good amount of time (more than once) scratching my head trying to understand this loop.
💬 ryanofsky commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#issuecomment-2374578102)
> I wonder if the Deserializable concept could just live in the `serialize.h` header. It seems like it could be generally useful?

Definitely could be. Would be happy to move it if I need to update this again. Reason I didn't move is after the last update is that it's not really consistent with other concepts there, because it not used in that file, and it doesn't take a stream parameter. But I could move it and make it more consistent.

>
> My `clang-format` also wants to indent the `requ
...
⚠️ fanquake opened an issue: "assumeutxo: not syncing the snapshot chainstate"
(https://github.com/bitcoin/bitcoin/issues/30971)
Testing `-loadtxoutset` on node running master (39219fe145e5), with Sjors snapshot (height 840000). After it finished validating the snapshot, it (so far), has just not started syncing the snapshot chain, only the background chain is progressing. It also seems like the cache resizing has note worked correctly?
```bash
./build/src/bitcoind
2024-09-25T15:53:23Z Bitcoin Core version v28.99.0-39219fe145e5 (release build)
2024-09-25T15:53:23Z Script verification uses 9 additional threads
2024-0
...
📝 BrandonOdiwuor opened a pull request: "Wallet listreceivedby fix"
(https://github.com/bitcoin/bitcoin/pull/30972)
Fixes https://github.com/bitcoin/bitcoin/issues/16159,

This PR builds on https://github.com/bitcoin/bitcoin/pull/25973, fixing `listreceivedby*` RPCs by filtering out `send` addresses using `IsMine` (see https://github.com/bitcoin/bitcoin/pull/25973#discussion_r1269477246). It also breaks down the `listreceivedby` tests into subtests and adds a test to verify 'listreceivedby*' does not return `send` addresses
📝 BrandonOdiwuor converted_to_draft a pull request: "Wallet listreceivedby fix"
(https://github.com/bitcoin/bitcoin/pull/30972)
Fixes https://github.com/bitcoin/bitcoin/issues/16159,

This PR builds on https://github.com/bitcoin/bitcoin/pull/25973, fixing `listreceivedby*` RPCs by filtering out `send` addresses using `IsMine` (see https://github.com/bitcoin/bitcoin/pull/25973#discussion_r1269477246). It also breaks down the `listreceivedby` tests into subtests and adds a test to verify 'listreceivedby*' does not return `send` addresses
💬 average-gary commented on issue "Embedded ASMap data Tracking Issue":
(https://github.com/bitcoin/bitcoin/issues/28794#issuecomment-2374720112)
Is there no way to query AS Maps directly from the BGP tables?
💬 TheCharlatan commented on pull request "init: Remove retry for loop":
(https://github.com/bitcoin/bitcoin/pull/30968#issuecomment-2374760380)
Thank you for your suggestion @ryanofsky,

Updated 705105de4f84f2ce3bb1d754c88c32349e01bb3b -> e9d60af9889c12b4d427adefa53fd234e417f8f6 ([removeInitForLoop_0](https://github.com/TheCharlatan/bitcoin/tree/removeInitForLoop_0) -> [removeInitForLoop_1](https://github.com/TheCharlatan/bitcoin/tree/removeInitForLoop_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/removeInitForLoop_0..removeInitForLoop_1))

* Took @ryanofsky's [suggestion](https://github.com/bitcoin/bitcoin/pull/3096
...
💬 sdaftuar commented on pull request "Disable RBF Rule 2":
(https://github.com/bitcoin/bitcoin/pull/30964#issuecomment-2374765971)
You're right that rule 2 is broken. The issue is that our replacement rules are broken in many ways, and if we simply remove rule 2, the result would also be broken. The motivation for rule 2 was the idea that by introducing a new unconfirmed parent, the effective feerate at which the replacement transaction being validated would be mined might be much lower than the transactions it is evicting (say, if the new unconfirmed parent had a very low feerate). So by simply removing the rule, I thin
...
🤔 ismaelsadeeq reviewed a pull request: "Cluster linearization: separate tests from tests-of-tests"
(https://github.com/bitcoin/bitcoin/pull/30605#pullrequestreview-2328851273)
Code review bf1b1b09ca4dd18b7848fb1807df8533cf930df7

This is a nice refactor, really improve the cluster linearize test a lot.
💬 ismaelsadeeq commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1775578705)
I believe this new behavior is correct, as it reduces `iteration_left` when work is being done. This aligns with the behavior in `SearchCandidateFinder`.

On a side note, I think "iterations" might be a misnomer here. A more accurate term could be "maximum optimization" or "optimizations left".
💬 ismaelsadeeq commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1775638585)
This can be in the same loop above?

Why not just compare with the result of ancestor finder?
💬 ismaelsadeeq commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1775677456)
Should we also assert that the transactions in the set are the same?
💬 ismaelsadeeq commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1775651129)
We could wrap this in a helper to be dry, as same code is used above.
💬 ismaelsadeeq commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1775682420)
Maybe indicate that we only compare `SimpleCandidateFinder` with `ExhaustiveCandidateFinder` when we optimally found a subset and the cluster size is small.
🤔 ismaelsadeeq reviewed a pull request: "init: Remove retry for loop"
(https://github.com/bitcoin/bitcoin/pull/30968#pullrequestreview-2329121486)
Nice Concept ACK
💬 ismaelsadeeq commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1775732905)
could be in it's own commit?