✅ evansmj closed a pull request: "doc: Add Python install notes to build-osx.md."
(https://github.com/bitcoin/bitcoin/pull/27728)
(https://github.com/bitcoin/bitcoin/pull/27728)
💬 TheCharlatan commented on pull request "Return EXIT_FAILURE on post-init fatal errors":
(https://github.com/bitcoin/bitcoin/pull/27708#discussion_r1207346355)
> Which.. isn't bad as it will remove another ui_interface dependency.
Can you elaborate on this point? `AbortNode` still relies on `ui_interface` from what I can tell.
From my current understanding both the `StartShutdown` in `ThreadImport` and in the check are call shutdown and node `AbortNode`, because `BlockImport` might run before the ui is available (this is me guessing out of historical reasons). I am not sold on this point though, since this runs in another thread and we also prov
...
(https://github.com/bitcoin/bitcoin/pull/27708#discussion_r1207346355)
> Which.. isn't bad as it will remove another ui_interface dependency.
Can you elaborate on this point? `AbortNode` still relies on `ui_interface` from what I can tell.
From my current understanding both the `StartShutdown` in `ThreadImport` and in the check are call shutdown and node `AbortNode`, because `BlockImport` might run before the ui is available (this is me guessing out of historical reasons). I am not sold on this point though, since this runs in another thread and we also prov
...
📝 furszy opened a pull request: "Fix rpc_getblockfrompeer intermittency by introducing 'getblockfileinfo' RPC command"
(https://github.com/bitcoin/bitcoin/pull/27770)
Attempt to fix #27749.
Implemented a `getblockfileinfo` RPC command to obtain block files related data.
So it can be used to fix the intermittency presented inside `rpc_getblockfrompeer.py`.
#27749 issue is the number of blocks contained by the first block file, which is not fixed
and can vary depending on the inner block sizes (what is fixed is the total size of the file
but not the number of blocks that the file contains).
Basically, `pruneblockchain(height)` tries to prune blocks
...
(https://github.com/bitcoin/bitcoin/pull/27770)
Attempt to fix #27749.
Implemented a `getblockfileinfo` RPC command to obtain block files related data.
So it can be used to fix the intermittency presented inside `rpc_getblockfrompeer.py`.
#27749 issue is the number of blocks contained by the first block file, which is not fixed
and can vary depending on the inner block sizes (what is fixed is the total size of the file
but not the number of blocks that the file contains).
Basically, `pruneblockchain(height)` tries to prune blocks
...
💬 furszy commented on issue "rpc_getblockfrompeer.py intermittent failure: assert_equal(pruneheight, 248); not(249 == 248)":
(https://github.com/bitcoin/bitcoin/issues/27749#issuecomment-1564966458)
I wasn't able to reproduce it but my spider sense tells me that it will be fixed by #27770.
(https://github.com/bitcoin/bitcoin/issues/27749#issuecomment-1564966458)
I wasn't able to reproduce it but my spider sense tells me that it will be fixed by #27770.
💬 jamesob commented on pull request "assumeutxo: net_processing changes":
(https://github.com/bitcoin/bitcoin/pull/24008#issuecomment-1564974163)
Closing in lieu of some simpler changes that @sdaftuar has written, currently integrated in #27596.
(https://github.com/bitcoin/bitcoin/pull/24008#issuecomment-1564974163)
Closing in lieu of some simpler changes that @sdaftuar has written, currently integrated in #27596.
✅ jamesob closed a pull request: "assumeutxo: net_processing changes"
(https://github.com/bitcoin/bitcoin/pull/24008)
(https://github.com/bitcoin/bitcoin/pull/24008)
💬 furszy commented on pull request "Return EXIT_FAILURE on post-init fatal errors":
(https://github.com/bitcoin/bitcoin/pull/27708#discussion_r1207406938)
> > Which.. isn't bad as it will remove another ui_interface dependency.
>
> Can you elaborate on this point? `AbortNode` still relies on `ui_interface` from what I can tell.
Ok, I confused you. Sorry.
The ui_interface dependency that can be removed is the `base.cpp` one. Thus why said to "bubble up" the error string up to `init.cpp` instead of calling `InitError` from the index base class `Start` method.
> From my current understanding both the `StartShutdown` in `ThreadImport` and
...
(https://github.com/bitcoin/bitcoin/pull/27708#discussion_r1207406938)
> > Which.. isn't bad as it will remove another ui_interface dependency.
>
> Can you elaborate on this point? `AbortNode` still relies on `ui_interface` from what I can tell.
Ok, I confused you. Sorry.
The ui_interface dependency that can be removed is the `base.cpp` one. Thus why said to "bubble up" the error string up to `init.cpp` instead of calling `InitError` from the index base class `Start` method.
> From my current understanding both the `StartShutdown` in `ThreadImport` and
...
💬 ishaanam commented on pull request "wallet: improve IBD sync time by skipping block scanning prior birth time":
(https://github.com/bitcoin/bitcoin/pull/27469#issuecomment-1565012825)
re-ACK 82bb7831fa6052620998c7eef47e48ed594248a8
(https://github.com/bitcoin/bitcoin/pull/27469#issuecomment-1565012825)
re-ACK 82bb7831fa6052620998c7eef47e48ed594248a8
💬 LarryRuane commented on pull request "util: generalize accounting of system-allocated memory in pool resource":
(https://github.com/bitcoin/bitcoin/pull/27748#issuecomment-1565039269)
I decided to reopen this; I think it's a good generalization improvement to the pool allocator. See the description for details. Thanks, @martinus, for the suggested improvement!
(https://github.com/bitcoin/bitcoin/pull/27748#issuecomment-1565039269)
I decided to reopen this; I think it's a good generalization improvement to the pool allocator. See the description for details. Thanks, @martinus, for the suggested improvement!
💬 LarryRuane commented on pull request "util: generalize accounting of system-allocated memory in pool resource":
(https://github.com/bitcoin/bitcoin/pull/27748#issuecomment-1565042192)
GitHub isn't allowing me to reopen this; @achow101 could you re-open this for me, please? Or, if you'd prefer, I could make a new PR. Thanks!
TL;DR on this PR's history: When I created this PR, I thought I was fixing a bug. But @martinus pointed out that there is no bug, so I closed this PR. I then decided this would be a worthwhile improvement after all (although of course not trying to fix a bug), so I'd like it to be reopened.
(https://github.com/bitcoin/bitcoin/pull/27748#issuecomment-1565042192)
GitHub isn't allowing me to reopen this; @achow101 could you re-open this for me, please? Or, if you'd prefer, I could make a new PR. Thanks!
TL;DR on this PR's history: When I created this PR, I thought I was fixing a bug. But @martinus pointed out that there is no bug, so I closed this PR. I then decided this would be a worthwhile improvement after all (although of course not trying to fix a bug), so I'd like it to be reopened.
💬 amitiuttarwar commented on pull request "addrman: select addresses by network follow-up":
(https://github.com/bitcoin/bitcoin/pull/27745#discussion_r1207469865)
should be ok now (both here and in `GetEntry`)
(https://github.com/bitcoin/bitcoin/pull/27745#discussion_r1207469865)
should be ok now (both here and in `GetEntry`)
💬 achow101 commented on pull request "wallet: improve IBD sync time by skipping block scanning prior birth time":
(https://github.com/bitcoin/bitcoin/pull/27469#issuecomment-1565133194)
re-ACK 82bb7831fa6052620998c7eef47e48ed594248a8
(https://github.com/bitcoin/bitcoin/pull/27469#issuecomment-1565133194)
re-ACK 82bb7831fa6052620998c7eef47e48ed594248a8
💬 amitiuttarwar commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1565134780)
> Ah, I think the problem with the test is that `AddRandomOutboundPeer` creates random IPv4 addresses, and sometimes these aren't routable (e.g. protected ranges), so that they are not assigned to `NET_IPV4` but `NET_UNROUTABLE`, which then makes the test fail.
great find @mzumsande ! I was able to reproduce & observe this behavior, so I added a loop in `AddRandomOutboundPeer` to try again if the selected address is not routable. hopefully this works, I ran the current version >1000 times and
...
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1565134780)
> Ah, I think the problem with the test is that `AddRandomOutboundPeer` creates random IPv4 addresses, and sometimes these aren't routable (e.g. protected ranges), so that they are not assigned to `NET_IPV4` but `NET_UNROUTABLE`, which then makes the test fail.
great find @mzumsande ! I was able to reproduce & observe this behavior, so I added a loop in `AddRandomOutboundPeer` to try again if the selected address is not routable. hopefully this works, I ran the current version >1000 times and
...
🚀 achow101 merged a pull request: "wallet: improve IBD sync time by skipping block scanning prior birth time"
(https://github.com/bitcoin/bitcoin/pull/27469)
(https://github.com/bitcoin/bitcoin/pull/27469)
💬 ishaanam commented on pull request "wallet: when a block is disconnected, update transactions that are no longer conflicted":
(https://github.com/bitcoin/bitcoin/pull/27145#issuecomment-1565156770)
> If mapTxSpends.count(outpoint) == 1, the outpoint was spent only once, so the wallet shouldn't contain any conflicting tx.
This is not necessarily always true, because even though the conflicted wallet transaction must always have its spends added to `mapTxSpends`, this is not the case for all conflicting transactions.
For example, let's say that Alice has a utxo. She creates a transaction, tx1, spending this utxo to Bob. `AddToSpends` will get called on Bob's node for this transaction
...
(https://github.com/bitcoin/bitcoin/pull/27145#issuecomment-1565156770)
> If mapTxSpends.count(outpoint) == 1, the outpoint was spent only once, so the wallet shouldn't contain any conflicting tx.
This is not necessarily always true, because even though the conflicted wallet transaction must always have its spends added to `mapTxSpends`, this is not the case for all conflicting transactions.
For example, let's say that Alice has a utxo. She creates a transaction, tx1, spending this utxo to Bob. `AddToSpends` will get called on Bob's node for this transaction
...
💬 mmienko commented on pull request "update osx build docs":
(https://github.com/bitcoin/bitcoin/pull/27769#issuecomment-1565181500)
> Thanks, however steps for how to (re-)install your package manager...not something we want to maintain.
That's a fair point and I agree. Appreciate the review. If I see issues for macOS like this, I'll leave a comment there.
(https://github.com/bitcoin/bitcoin/pull/27769#issuecomment-1565181500)
> Thanks, however steps for how to (re-)install your package manager...not something we want to maintain.
That's a fair point and I agree. Appreciate the review. If I see issues for macOS like this, I'll leave a comment there.
✅ mmienko closed a pull request: "update osx build docs"
(https://github.com/bitcoin/bitcoin/pull/27769)
(https://github.com/bitcoin/bitcoin/pull/27769)
💬 fanquake commented on pull request "fuzz: Change LIMIT_TO_MESSAGE_TYPE from a compile-time to a run-time setting":
(https://github.com/bitcoin/bitcoin/pull/27766#issuecomment-1565301478)
Nice. Should help with oss-fuzz disk issues. Related to https://github.com/bitcoin-core/qa-assets/pull/131
(https://github.com/bitcoin/bitcoin/pull/27766#issuecomment-1565301478)
Nice. Should help with oss-fuzz disk issues. Related to https://github.com/bitcoin-core/qa-assets/pull/131
🚀 fanquake merged a pull request: "fuzz: Change LIMIT_TO_MESSAGE_TYPE from a compile-time to a run-time setting"
(https://github.com/bitcoin/bitcoin/pull/27766)
(https://github.com/bitcoin/bitcoin/pull/27766)
💬 brunoerg commented on pull request "fuzz: Change LIMIT_TO_MESSAGE_TYPE from a compile-time to a run-time setting":
(https://github.com/bitcoin/bitcoin/pull/27766#issuecomment-1565384526)
post-merge ACK 1111c9ac97ca0f0afeb5df45bc0970b761c3c9ff
(https://github.com/bitcoin/bitcoin/pull/27766#issuecomment-1565384526)
post-merge ACK 1111c9ac97ca0f0afeb5df45bc0970b761c3c9ff