Bitcoin Core Github
42 subscribers
126K links
Download Telegram
💬 ryanofsky commented on pull request "test: interface_ipc.py minor fixes and cleanup":
(https://github.com/bitcoin/bitcoin/pull/34003#discussion_r2599716362)
re: https://github.com/bitcoin/bitcoin/pull/34003#discussion_r2598844209

> I think the original intention here was to demonstrate that `generate` interrupts an ongoing `waitNext()`.
>
> Now it only shows that `waitNext()` returns immediately if the tip has changes before calling. Though perhaps it always did that by accident.

Yes behavior should be unchanged. Original code was:

```python
waitnext = template.result.waitNext(ctx, waitoptions)
self.generate(self.nodes[0], 1)
template
...
💬 Sjors commented on pull request "validation: Remove min_pow_checked arg in ProcessNewBlockHeaders":
(https://github.com/bitcoin/bitcoin/pull/34022#issuecomment-3628534056)
This was intentional, as explained in the commit message of ed6cddd98e32263fc116a4380af6d66da20da990 (part of #25717):

> This commit adds a new argument to `AcceptBlockHeader()` so that we can ensure that all call-sites which might cause a new header to be accepted into memory have to grapple with the question of whether the header is safe to accept, or needs further validation.

Not sure if we need to keep it forever.

cc @sdaftuar
💬 Sjors commented on pull request "validation: Remove min_pow_checked arg in ProcessNewBlockHeaders":
(https://github.com/bitcoin/bitcoin/pull/34022#discussion_r2599761636)
Maybe something loosely based on the original commit message:

> all call-sites which might cause a new header to be accepted into memory have to grapple with the question of whether the header is safe to accept, or needs further validation
🤔 mzumsande reviewed a pull request: "Broadcast own transactions only via short-lived Tor or I2P connections"
(https://github.com/bitcoin/bitcoin/pull/29415#pullrequestreview-3553664703)
re-ACK https://github.com/bitcoin/bitcoin/commit/f391296b17a755153960e0afc736df37d1d5fb1e
💬 instagibbs commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#discussion_r2590419145)
do you mean pick a chunk randomly, then observer the dependency/dependee list, and pick the corresponding lowers/highest feerate one for activation?
💬 instagibbs commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#discussion_r2590509887)
```Suggestion
// If we started from a topological input, the resulting feerate diagram cannot be worse or incomparable.
```
💬 instagibbs commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#discussion_r2593658728)
```Suggestion
// De-activate it otherwise.
```
💬 instagibbs commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#discussion_r2590318854)
```Suggestion
* The collection of all spanning trees for the entire cluster form a spanning forest. In the extreme each
```
💬 instagibbs commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#discussion_r2590522916)
85cff48b782538ed5a1c980015dd01cb32986b66

straggling AncestorCandidateFinder reference
💬 instagibbs commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#discussion_r2590327658)
> or equal

Need to understand why, delving seems to suggest the more "natural" choice https://delvingbitcoin.org/t/spanning-forest-cluster-linearization/1419 "If there is an inactive dependency, where the parent and child are in distinct chunks, and the child chunk has higher feerate than the parent chunk, make the dependency active, merging the two chunks."
💬 sipa commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#discussion_r2599895617)
If you don't merge along equal-feerate dependencies, you can end up in a situation with two equal-feerate "chunks" that depend on each other.

E.g. the AC and BD "chunks" in:

```mermaid
graph BT;
A["A: 1/1"];
B["B: 1/1"];
C["C: 1/1"];
D["D: 1/1"];
C --> A;
C -.-> B;
D -.-> A;
D --> B;
```

The solution is an initial phase where you don't care about splitting equal-feerate parts from one another (this PR), and then a secondary phase where you split up chunks in their minimal comp
...
🤔 mzumsande reviewed a pull request: "test: p2p: check that peer's announced starting height is remembered"
(https://github.com/bitcoin/bitcoin/pull/33990#pullrequestreview-3553772985)
I agree that it isn't useful, and depreciation would makes sense in my opinion. If we decide not to do that for some reason, a test makes sense though.
💬 sipa commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#discussion_r2599927986)
Yeah, exactly.

* Pick a random chunk (from the list you maintain of chunks that need to be considered still)
* See if it can be merged upwards or downwards, and:
* If so, merge with its max-feerate-difference dependency/depender, and add the resulting chunk to list.
* If not, remove the chunk from the list.
💬 ajtowns commented on issue "Net split meta issue":
(https://github.com/bitcoin/bitcoin/issues/33958#issuecomment-3629082728)
> > For the last point, I think there are two main principles we should aim for: (a) minimising the movement and reparsing of data, and (b) minimising the locking and (potentially) maximising the cache performance when we send/receive data. Currently, when we send data and don't need to block, we:

> I'm all for these goals, but I consider almost all of those things to be premature optimizations compared to the real-world bottlenecks that we currently see.

The single biggest bottleneck I see to
...
👍 sedited approved a pull request: "[IBD] coins: reduce lookups in dbcache layer propagation"
(https://github.com/bitcoin/bitcoin/pull/33602#pullrequestreview-3554048589)
ACK 0ac969cddfdba52f7947e9b140ef36e2b19c2c41
💬 sedited commented on pull request "[IBD] coins: reduce lookups in dbcache layer propagation":
(https://github.com/bitcoin/bitcoin/pull/33602#discussion_r2600124064)
Why not address these TODOs in this PR? Seems like all that needs to be done is pick the test changes from Andrew's commit?
💬 l0rinc commented on pull request "[IBD] coins: reduce lookups in dbcache layer propagation":
(https://github.com/bitcoin/bitcoin/pull/33602#discussion_r2600248073)
We have tried fixing that a few times, a lot of tests need to be updated, see [`320482e` (#30673)](https://github.com/bitcoin/bitcoin/pull/30673/commits/320482e860cb6834984c0c52643021496e57a655#r2206103568) and it scared people and we didn't get reviews for that, so we're doing it in smaller chunks now.

The TODO makes sense here since it documents that the current approach isn't a pessimization, it favors the only case that can happen in reality.
💬 l0rinc commented on pull request "[IBD] coins: reduce lookups in dbcache layer propagation":
(https://github.com/bitcoin/bitcoin/pull/33602#issuecomment-3629137740)
thanks for the reviews!
rfm?
💬 mzumsande commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2600302551)
I have been thinking about removing this call from `ABCStep` for a while, because it doesn't seem necessary:
During `ConnectTip()`, we call `InvalidBlockFound()`, which calls `InvalidChainFound()` for the block that fails validation. The additional call to `InvalidChainFound()` with the highest connectable block from the chain (which could be a successor of invalid block, or the same block again) can lead to additional unnecessary work and duplicate logging. The only part from `InvalidChainFoun
...
💬 Satoshin-Btc commented on issue "Net split meta issue":
(https://github.com/bitcoin/bitcoin/issues/33958#issuecomment-3629209877)
so i agree with the things you pointed out. the rushed merger was a complete mess the last time in 2022 and didn't bode well either. i get the node and process thread issue for validated peers for txs blocks and so forth. my only thought on solution is something similar to the net split but that causes its own issues. so with dedicated nodes back to where the we're before allowing calls from the different containers helped the process a lot more than the jumbled up system we are dealing with. th
...
💬 sedited commented on pull request "validation: Remove min_pow_checked arg in ProcessNewBlockHeaders":
(https://github.com/bitcoin/bitcoin/pull/34022#issuecomment-3629213020)
Updated beff6ff964c5c1b4a96af89daf6b2ddaede3464a -> 8ca9997e48bb2067ea83fabb1c640af72178f97c ([rmMinPowChecked_0](https://github.com/TheCharlatan/bitcoin/tree/rmMinPowChecked_0) -> [rmMinPowChecked_1](https://github.com/TheCharlatan/bitcoin/tree/rmMinPowChecked_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/rmMinPowChecked_0..rmMinPowChecked_1))

* Addressed [comment](https://github.com/bitcoin/bitcoin/pull/34022#discussion_r2599070189), and [comment](https://github.com/bitcoin/
...