💬 vasild commented on pull request "net processing: avoid serving non-announced txs as a result of a MEMPOOL message":
(https://github.com/bitcoin/bitcoin/pull/27602#issuecomment-1545379692)
No, wait! This PR changes both Case1 and Case2, but Case1 is only about things happening in the same second.
What I tried yesterday:
* Run the newly added tests, they were passing, of course. Cool!
* Revert the `net_processing.cpp` changes, both tests `Request the irrelevant transaction even though it has not being offered to us` and `Create a third transaction (that was therefore not part of the mempool message) and try to...` failed. Good! To reach the second test I disabled the `assert`
...
(https://github.com/bitcoin/bitcoin/pull/27602#issuecomment-1545379692)
No, wait! This PR changes both Case1 and Case2, but Case1 is only about things happening in the same second.
What I tried yesterday:
* Run the newly added tests, they were passing, of course. Cool!
* Revert the `net_processing.cpp` changes, both tests `Request the irrelevant transaction even though it has not being offered to us` and `Create a third transaction (that was therefore not part of the mempool message) and try to...` failed. Good! To reach the second test I disabled the `assert`
...
💬 fanquake commented on pull request "Introduce field element and group element classes to test_framework/key.py":
(https://github.com/bitcoin/bitcoin/pull/26222#issuecomment-1545399444)
cc @real-or-random
(https://github.com/bitcoin/bitcoin/pull/26222#issuecomment-1545399444)
cc @real-or-random
💬 willcl-ark commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#issuecomment-1545399521)
reACK af86462
Now using const reference in `parse()`.
(https://github.com/bitcoin/bitcoin/pull/27101#issuecomment-1545399521)
reACK af86462
Now using const reference in `parse()`.
💬 vasild commented on pull request "net processing: avoid serving non-announced txs as a result of a MEMPOOL message":
(https://github.com/bitcoin/bitcoin/pull/27602#issuecomment-1545400201)
Looking at the comment of `CRollingBloomFilter` if `m_recently_announced_invs` is changed from `3500` to `300000` then its size would increase from 37KB to 3MB. I guess that is too much and maybe was the reason why `MEMPOOL` replies don't use `m_recently_announced_invs` and instead use the `m_last_mempool_req` timing mechanism.
(https://github.com/bitcoin/bitcoin/pull/27602#issuecomment-1545400201)
Looking at the comment of `CRollingBloomFilter` if `m_recently_announced_invs` is changed from `3500` to `300000` then its size would increase from 37KB to 3MB. I guess that is too much and maybe was the reason why `MEMPOOL` replies don't use `m_recently_announced_invs` and instead use the `m_last_mempool_req` timing mechanism.
💬 fanquake commented on pull request "BIP324: ElligatorSwift integrations":
(https://github.com/bitcoin/bitcoin/pull/27479#issuecomment-1545400377)
cc @stratospher
(https://github.com/bitcoin/bitcoin/pull/27479#issuecomment-1545400377)
cc @stratospher
📝 Riahiamirreza opened a pull request: "rpc: show P2(W)SH redeemScript in getrawtransaction"
(https://github.com/bitcoin/bitcoin/pull/27637)
This a work in progress PR. I'll squash all my commits at the end to make commits atomic (as mentioned [here](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#committing-patches))
It's an effort to solve this issue https://github.com/bitcoin/bitcoin/issues/27391.
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https
...
(https://github.com/bitcoin/bitcoin/pull/27637)
This a work in progress PR. I'll squash all my commits at the end to make commits atomic (as mentioned [here](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#committing-patches))
It's an effort to solve this issue https://github.com/bitcoin/bitcoin/issues/27391.
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https
...
💬 Sjors commented on pull request "Improve performance of p2p inv to send queues":
(https://github.com/bitcoin/bitcoin/pull/27610#discussion_r1192111163)
For a followup it would be could to clarify "should be considered" _for what_. It's somewhat confusing to reason about because the call site reverses a and b.
The first case in the comment is "a is not in the mempool, but b is", but the line of code checks if b is not in the mempool.
(https://github.com/bitcoin/bitcoin/pull/27610#discussion_r1192111163)
For a followup it would be could to clarify "should be considered" _for what_. It's somewhat confusing to reason about because the call site reverses a and b.
The first case in the comment is "a is not in the mempool, but b is", but the line of code checks if b is not in the mempool.
✅ Riahiamirreza closed a pull request: "rpc: show P2(W)SH redeemScript in getrawtransaction"
(https://github.com/bitcoin/bitcoin/pull/27637)
(https://github.com/bitcoin/bitcoin/pull/27637)
📝 Riahiamirreza opened a pull request: "rpc: show P2(W)SH redeemScript in getrawtransaction #27637"
(https://github.com/bitcoin/bitcoin/pull/27638)
This a work in progress PR. I'll squash all my commits at the end to make commits atomic (as mentioned [here](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#committing-patches))
It's an effort to solve this issue https://github.com/bitcoin/bitcoin/issues/27391.
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
h
...
(https://github.com/bitcoin/bitcoin/pull/27638)
This a work in progress PR. I'll squash all my commits at the end to make commits atomic (as mentioned [here](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#committing-patches))
It's an effort to solve this issue https://github.com/bitcoin/bitcoin/issues/27391.
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
h
...
🚀 fanquake merged a pull request: "[23.2] Backports for rc1"
(https://github.com/bitcoin/bitcoin/pull/27624)
(https://github.com/bitcoin/bitcoin/pull/27624)
📝 fanquake locked a pull request: "rpc: show P2(W)SH redeemScript in getrawtransaction"
(https://github.com/bitcoin/bitcoin/pull/27637)
This a work in progress PR. I'll squash all my commits at the end to make commits atomic (as mentioned [here](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#committing-patches))
It's an effort to solve this issue https://github.com/bitcoin/bitcoin/issues/27391.
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https
...
(https://github.com/bitcoin/bitcoin/pull/27637)
This a work in progress PR. I'll squash all my commits at the end to make commits atomic (as mentioned [here](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#committing-patches))
It's an effort to solve this issue https://github.com/bitcoin/bitcoin/issues/27391.
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https
...
💬 MarcoFalke commented on pull request "Improve performance of p2p inv to send queues":
(https://github.com/bitcoin/bitcoin/pull/27610#discussion_r1192123432)
> The first case in the comment is "a is not in the mempool, but b is", but the line of code checks if b is not in the mempool.
If b is not in the mempool, the function must return false, see https://github.com/bitcoin/bitcoin/pull/27610#discussion_r1191235815
If b is in the mempool, the function will continue over the early return `false`, and checks to see if a is missing, in which case it should be considered sooner.
(https://github.com/bitcoin/bitcoin/pull/27610#discussion_r1192123432)
> The first case in the comment is "a is not in the mempool, but b is", but the line of code checks if b is not in the mempool.
If b is not in the mempool, the function must return false, see https://github.com/bitcoin/bitcoin/pull/27610#discussion_r1191235815
If b is in the mempool, the function will continue over the early return `false`, and checks to see if a is missing, in which case it should be considered sooner.
💬 joostjager commented on pull request "rpc: allow submitpackage to be called outside of regtest":
(https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1545439229)
>With the "tree only" topology requirement I feel more comfortable having this branch exist, but still don't think this is something to merge yet / backport.
Do you think this is not ready to merge / backport yet because of the unexpected edge cases that showed up with the previous, less restrictive iteration, leading to the expectation that something might show up for "tree only" too? Or is there already a known issue for "tree only"?
(https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1545439229)
>With the "tree only" topology requirement I feel more comfortable having this branch exist, but still don't think this is something to merge yet / backport.
Do you think this is not ready to merge / backport yet because of the unexpected edge cases that showed up with the previous, less restrictive iteration, leading to the expectation that something might show up for "tree only" too? Or is there already a known issue for "tree only"?
💬 dergoegge commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1192138027)
This could be a `else if` of the outer if statement. That would avoid the nesting here.
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1192138027)
This could be a `else if` of the outer if statement. That would avoid the nesting here.
💬 vasild commented on pull request "p2p: skip netgroup diversity follow-up":
(https://github.com/bitcoin/bitcoin/pull/27467#issuecomment-1545496847)
> I don't think it's useful to have these kind of PRs
Ok, this is your opinion. Sometimes I feel the same for some PRs. My reaction is then to ignore them - don't spend _my_ time on them if _I_ think it is not worth it. But I don't try to impose my opinion on others - if somebody was thinking it is worth to open a PR and somebody else was thinking it is worth to review, then let it be. Those other people have different opinion than mine and I am not going to tell them what to work on or what
...
(https://github.com/bitcoin/bitcoin/pull/27467#issuecomment-1545496847)
> I don't think it's useful to have these kind of PRs
Ok, this is your opinion. Sometimes I feel the same for some PRs. My reaction is then to ignore them - don't spend _my_ time on them if _I_ think it is not worth it. But I don't try to impose my opinion on others - if somebody was thinking it is worth to open a PR and somebody else was thinking it is worth to review, then let it be. Those other people have different opinion than mine and I am not going to tell them what to work on or what
...
💬 kroese commented on issue "CPU DoS on mainnet in debug mode":
(https://github.com/bitcoin/bitcoin/issues/27586#issuecomment-1545510083)
> You probably need to restart the node to clear it up after it's started spinning
You were right.. restarting it dropped CPU usage from 100 percent to about 3 percent immediately. I hope it will stay this way as rc3 (which should fix it) still hasn't been released.
(https://github.com/bitcoin/bitcoin/issues/27586#issuecomment-1545510083)
> You probably need to restart the node to clear it up after it's started spinning
You were right.. restarting it dropped CPU usage from 100 percent to about 3 percent immediately. I hope it will stay this way as rc3 (which should fix it) still hasn't been released.
👍 vasild approved a pull request: "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`"
(https://github.com/bitcoin/bitcoin/pull/26261#pullrequestreview-1424217567)
ACK 4eee95e57bb6f773bcaeb405bca949f158a62134
(https://github.com/bitcoin/bitcoin/pull/26261#pullrequestreview-1424217567)
ACK 4eee95e57bb6f773bcaeb405bca949f158a62134
💬 fanquake commented on pull request "build: Fix shared lib linking for darwin with lld":
(https://github.com/bitcoin/bitcoin/pull/27628#issuecomment-1545559220)
Guix Build:
```bash
0e6772d00e899aea446bbda37b198142812b95830e4c8f52bde9992efd8bc570 guix-build-d576d69e139b/output/aarch64-linux-gnu/SHA256SUMS.part
f0b4ac500389b012f78c20043adef975e2a33b8e079bbee07b3e4c130c4f67a9 guix-build-d576d69e139b/output/aarch64-linux-gnu/bitcoin-d576d69e139b-aarch64-linux-gnu-debug.tar.gz
51e611952c543a7ad9933934c6b2160703ae2ebe36804b30446a874e49f85130 guix-build-d576d69e139b/output/aarch64-linux-gnu/bitcoin-d576d69e139b-aarch64-linux-gnu.tar.gz
feab71eebaf317b8
...
(https://github.com/bitcoin/bitcoin/pull/27628#issuecomment-1545559220)
Guix Build:
```bash
0e6772d00e899aea446bbda37b198142812b95830e4c8f52bde9992efd8bc570 guix-build-d576d69e139b/output/aarch64-linux-gnu/SHA256SUMS.part
f0b4ac500389b012f78c20043adef975e2a33b8e079bbee07b3e4c130c4f67a9 guix-build-d576d69e139b/output/aarch64-linux-gnu/bitcoin-d576d69e139b-aarch64-linux-gnu-debug.tar.gz
51e611952c543a7ad9933934c6b2160703ae2ebe36804b30446a874e49f85130 guix-build-d576d69e139b/output/aarch64-linux-gnu/bitcoin-d576d69e139b-aarch64-linux-gnu.tar.gz
feab71eebaf317b8
...
💬 rebroad commented on issue "assumeutxo":
(https://github.com/bitcoin/bitcoin/issues/15605#issuecomment-1545571482)
Regarding bootstrapping a pruned node from a full-node, the way I'd do it is to bitcoin-cli the running node to stop it accepting any more blocks or transactions, copy --reflink=always the block index and chainstate to a new bitcoind instance, bitcoin-cli the running node to resume accepting blocks and transactions, modify the new instance to use new ports, and enable pruning, and run it, wait for it to prune the block index, and then exit, and voila, I have a ready to copy bitcoind instance tha
...
(https://github.com/bitcoin/bitcoin/issues/15605#issuecomment-1545571482)
Regarding bootstrapping a pruned node from a full-node, the way I'd do it is to bitcoin-cli the running node to stop it accepting any more blocks or transactions, copy --reflink=always the block index and chainstate to a new bitcoind instance, bitcoin-cli the running node to resume accepting blocks and transactions, modify the new instance to use new ports, and enable pruning, and run it, wait for it to prune the block index, and then exit, and voila, I have a ready to copy bitcoind instance tha
...
💬 vasild commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#issuecomment-1545603904)
`a81fe4ff9b...f559067e27`: rebase due to conflicts, thanks for looking into this!
(https://github.com/bitcoin/bitcoin/pull/26812#issuecomment-1545603904)
`a81fe4ff9b...f559067e27`: rebase due to conflicts, thanks for looking into this!