💬 instagibbs commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204554984)
Not anymore, we completely forget we expect it from a peer now. No disconnect should occur.
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204554984)
Not anymore, we completely forget we expect it from a peer now. No disconnect should occur.
💬 Sjors commented on issue "Spurious (?) valgrind failure for p2p_compactblocks.py":
(https://github.com/bitcoin/bitcoin/issues/27741#issuecomment-1561678286)
I compiled on cce96182ba2457335868c65dc16b081c3dee32ee, but can reproduce on master. Updated description.
(https://github.com/bitcoin/bitcoin/issues/27741#issuecomment-1561678286)
I compiled on cce96182ba2457335868c65dc16b081c3dee32ee, but can reproduce on master. Updated description.
💬 denavila commented on pull request "Deniability - a tool to automatically improve coin ownership privacy":
(https://github.com/bitcoin-core/gui/pull/733#issuecomment-1561680006)
> It seems like a feature like this would be better to integrate into the wallet directly rather than something that is done through and managed by the GUI.
Do you mean moving some of the core logic inside the interfaces::Wallet API, or the CWallet implementation?
Yes, certainly, that could have benefits. I had to workaround certain things that weren't in the Wallet API (namely address reservation, bump fee handling etc). So that would clean up the wallet side for sure.
Since this was m
...
(https://github.com/bitcoin-core/gui/pull/733#issuecomment-1561680006)
> It seems like a feature like this would be better to integrate into the wallet directly rather than something that is done through and managed by the GUI.
Do you mean moving some of the core logic inside the interfaces::Wallet API, or the CWallet implementation?
Yes, certainly, that could have benefits. I had to workaround certain things that weren't in the Wallet API (namely address reservation, bump fee handling etc). So that would clean up the wallet side for sure.
Since this was m
...
💬 instagibbs commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204558056)
if it's 1, that's fine, it just won't allow additional slots
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204558056)
if it's 1, that's fine, it just won't allow additional slots
💬 instagibbs commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204558678)
oops, indeed, looks like we'll send off bogus getblocktxns here, if it comes from a preferred peer that's not first. needs a fix.
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204558678)
oops, indeed, looks like we'll send off bogus getblocktxns here, if it comes from a preferred peer that's not first. needs a fix.
💬 instagibbs commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204561198)
oops, indeed, looks like we'll send off bogus getblocktxns here, if it comes from a preferred peer that's not first. needs a fix. Wasted bandwidth and should result in "Peer %d sent us block transactions for block we weren't expecting\n"
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204561198)
oops, indeed, looks like we'll send off bogus getblocktxns here, if it comes from a preferred peer that's not first. needs a fix. Wasted bandwidth and should result in "Peer %d sent us block transactions for block we weren't expecting\n"
💬 Sjors commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204563081)
That was already the case. The comment is there to point out that the current request will result in a disconnect (unless a new request is made on time).
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204563081)
That was already the case. The comment is there to point out that the current request will result in a disconnect (unless a new request is made on time).
💬 furszy commented on pull request "wallet: improve IBD sync time by skipping block scanning prior birth time":
(https://github.com/bitcoin/bitcoin/pull/27469#discussion_r1204567065)
> Is there a reason for the first commit?
It is because this is the only place where we set the chain timestamps to be in the past. In the form of
```
block_1_timestamp = genesis + 1
block_2_timestamp = median(genesis, block_1) + 1
block_3_timestamp = median(genesis, block_1, block_2) + 1
```
And in the wallet, we use the clock time to set the descriptors/keys creation time.
So, even when we create a wallet prior mining the chain, the wallet will always have a birth time after it. Wh
...
(https://github.com/bitcoin/bitcoin/pull/27469#discussion_r1204567065)
> Is there a reason for the first commit?
It is because this is the only place where we set the chain timestamps to be in the past. In the form of
```
block_1_timestamp = genesis + 1
block_2_timestamp = median(genesis, block_1) + 1
block_3_timestamp = median(genesis, block_1, block_2) + 1
```
And in the wallet, we use the clock time to set the descriptors/keys creation time.
So, even when we create a wallet prior mining the chain, the wallet will always have a birth time after it. Wh
...
💬 instagibbs commented on pull request "net, refactor: net_processing, add `ProcessCompactBlockTxns`":
(https://github.com/bitcoin/bitcoin/pull/26969#issuecomment-1561694264)
Code review ACK https://github.com/bitcoin/bitcoin/pull/26969/commits/355bc6098a7373feb1d59f9651a79e1477d22243
reviewed with `git diff master --color-moved-ws=ignore-all-space --color-moved=dimmed-zebra`. very straight forward set of changes.
We have a small bug that you could do in a fixup commit as well perhaps: https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204491894
(https://github.com/bitcoin/bitcoin/pull/26969#issuecomment-1561694264)
Code review ACK https://github.com/bitcoin/bitcoin/pull/26969/commits/355bc6098a7373feb1d59f9651a79e1477d22243
reviewed with `git diff master --color-moved-ws=ignore-all-space --color-moved=dimmed-zebra`. very straight forward set of changes.
We have a small bug that you could do in a fixup commit as well perhaps: https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204491894
💬 mzumsande commented on issue "Frequent "Timeout downloading block" with 24.1":
(https://github.com/bitcoin/bitcoin/issues/27705#issuecomment-1561696153)
> I did block the peer right after I posted the latest logs four days ago, and after that the block timeouts dropped to 2-3 per day, but like I mentioned in the bug report, since Core doesn't normally log the IP of disconnected incoming peers you pretty much have to enable debug=net logging to find the IP of the misbehaving peer. Which on this particular node is ~1GB of logs per 2-3 hours.
The reason why connections/disconnections aren't logged for inbound peers is that these events are remot
...
(https://github.com/bitcoin/bitcoin/issues/27705#issuecomment-1561696153)
> I did block the peer right after I posted the latest logs four days ago, and after that the block timeouts dropped to 2-3 per day, but like I mentioned in the bug report, since Core doesn't normally log the IP of disconnected incoming peers you pretty much have to enable debug=net logging to find the IP of the misbehaving peer. Which on this particular node is ~1GB of logs per 2-3 hours.
The reason why connections/disconnections aren't logged for inbound peers is that these events are remot
...
💬 brunoerg commented on pull request "net, refactor: net_processing, add `ProcessCompactBlockTxns`":
(https://github.com/bitcoin/bitcoin/pull/26969#issuecomment-1561696565)
> We have a small bug that you could do in a fixup commit as well perhaps: https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204491894
Gonna address it, thanks.
(https://github.com/bitcoin/bitcoin/pull/26969#issuecomment-1561696565)
> We have a small bug that you could do in a fixup commit as well perhaps: https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204491894
Gonna address it, thanks.
💬 instagibbs commented on pull request "net, refactor: net_processing, add `ProcessCompactBlockTxns`":
(https://github.com/bitcoin/bitcoin/pull/26969#issuecomment-1561703430)
Upon further consultation by @fanquake , I've been advised to just PR the change straight. keep this PR as is.
(https://github.com/bitcoin/bitcoin/pull/26969#issuecomment-1561703430)
Upon further consultation by @fanquake , I've been advised to just PR the change straight. keep this PR as is.
💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1204580821)
grandparent still not fixed size :facepalm: edited the assertion to be on the multiple of 10. I think it worked this time.
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1204580821)
grandparent still not fixed size :facepalm: edited the assertion to be on the multiple of 10. I think it worked this time.
💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1204581045)
Fixed
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1204581045)
Fixed
📝 instagibbs opened a pull request: "Unconditionally return when compact block status == READ_STATUS_FAILED"
(https://github.com/bitcoin/bitcoin/pull/27743)
Fixes https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204491894 which would have resulted in wasted bandwidth every once in a while.
(https://github.com/bitcoin/bitcoin/pull/27743)
Fixes https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204491894 which would have resulted in wasted bandwidth every once in a while.
💬 Sjors commented on pull request "net, refactor: net_processing, add `ProcessCompactBlockTxns`":
(https://github.com/bitcoin/bitcoin/pull/26969#issuecomment-1561716007)
Concept ACK. Light preference for splitting this commit into a pure move-only and the (much smaller) change of calling `ProcessCompactBlockTxns` instead of `ProcessMessage`. In the future that makes it easier to follow the history of changes.
(https://github.com/bitcoin/bitcoin/pull/26969#issuecomment-1561716007)
Concept ACK. Light preference for splitting this commit into a pure move-only and the (much smaller) change of calling `ProcessCompactBlockTxns` instead of `ProcessMessage`. In the future that makes it easier to follow the history of changes.
💬 achow101 commented on pull request "Deniability - a tool to automatically improve coin ownership privacy":
(https://github.com/bitcoin-core/gui/pull/733#issuecomment-1561716951)
> Do you mean moving some of the core logic inside the interfaces::Wallet API, or the CWallet implementation?
Inside of `CWallet`.
> Since this was my first Bitcoin Core change I decided to be more conservative and avoid API changes.
> Of course, if this PR is accepted, I'd be happy to move functionality into the API.
The API isn't public, so changing it isn't a big concern. Really the only consumer of it is the GUI.
Having it inside of `CWallet` will probably make this feature more
...
(https://github.com/bitcoin-core/gui/pull/733#issuecomment-1561716951)
> Do you mean moving some of the core logic inside the interfaces::Wallet API, or the CWallet implementation?
Inside of `CWallet`.
> Since this was my first Bitcoin Core change I decided to be more conservative and avoid API changes.
> Of course, if this PR is accepted, I'd be happy to move functionality into the API.
The API isn't public, so changing it isn't a big concern. Really the only consumer of it is the GUI.
Having it inside of `CWallet` will probably make this feature more
...
💬 theStack commented on issue "bitcoind hangs waiting for `g_requests.empty()`":
(https://github.com/bitcoin/bitcoin/issues/27722#issuecomment-1561718193)
> I was able to fix this with my branch here [Crypt-iQ@4fd7adb](https://github.com/Crypt-iQ/bitcoin/commit/4fd7adb30f584664421b6431bce8ebcbf7ceef2f) by adding a `evhttp_connection_set_closecb` callback. I didn't need to modify the logic related to `EV_READ` and no functional test errors are introduced on my machine
Can confirm that this patch fixes the issue on my side.
(https://github.com/bitcoin/bitcoin/issues/27722#issuecomment-1561718193)
> I was able to fix this with my branch here [Crypt-iQ@4fd7adb](https://github.com/Crypt-iQ/bitcoin/commit/4fd7adb30f584664421b6431bce8ebcbf7ceef2f) by adding a `evhttp_connection_set_closecb` callback. I didn't need to modify the logic related to `EV_READ` and no functional test errors are introduced on my machine
Can confirm that this patch fixes the issue on my side.
💬 Sjors commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204590378)
#27743
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204590378)
#27743
💬 Sjors commented on pull request "Unconditionally return when compact block status == READ_STATUS_FAILED":
(https://github.com/bitcoin/bitcoin/pull/27743#issuecomment-1561719535)
utACK d97269579769effbe6eec2303ea0cc3e396d3e0d
(https://github.com/bitcoin/bitcoin/pull/27743#issuecomment-1561719535)
utACK d97269579769effbe6eec2303ea0cc3e396d3e0d