💬 fanquake commented on issue "bitcoin-qt generates bad psbt":
(https://github.com/bitcoin/bitcoin/issues/27697#issuecomment-1553183578)
> I think https://github.com/bitcoin-core/gui/pull/687 was the fix.
Right. So fixed in 24.1 as well.
(https://github.com/bitcoin/bitcoin/issues/27697#issuecomment-1553183578)
> I think https://github.com/bitcoin-core/gui/pull/687 was the fix.
Right. So fixed in 24.1 as well.
💬 MarcoFalke commented on issue "bitcoin-qt generates bad psbt":
(https://github.com/bitcoin/bitcoin/issues/27697#issuecomment-1553192600)
What was the backport commit?
(https://github.com/bitcoin/bitcoin/issues/27697#issuecomment-1553192600)
What was the backport commit?
💬 MarcoFalke commented on issue "bitcoin-qt generates bad psbt":
(https://github.com/bitcoin/bitcoin/issues/27697#issuecomment-1553194643)
Backported in 0da38b6b0e74ee4479553d2bf611593fa51d54b3
(https://github.com/bitcoin/bitcoin/issues/27697#issuecomment-1553194643)
Backported in 0da38b6b0e74ee4479553d2bf611593fa51d54b3
💬 fanquake commented on issue "bitcoin-qt generates bad psbt":
(https://github.com/bitcoin/bitcoin/issues/27697#issuecomment-1553195015)
https://github.com/bitcoin/bitcoin/commit/0662105e884dce3eae9d3eb0b0d49600c1260b90
(https://github.com/bitcoin/bitcoin/issues/27697#issuecomment-1553195015)
https://github.com/bitcoin/bitcoin/commit/0662105e884dce3eae9d3eb0b0d49600c1260b90
💬 fanquake commented on pull request "Load PSBTs using istreambuf_iterator rather than istream_iterator":
(https://github.com/bitcoin-core/gui/pull/687#issuecomment-1553195889)
Removing label as this was backported: https://github.com/bitcoin/bitcoin/commit/0662105e884dce3eae9d3eb0b0d49600c1260b90.
(https://github.com/bitcoin-core/gui/pull/687#issuecomment-1553195889)
Removing label as this was backported: https://github.com/bitcoin/bitcoin/commit/0662105e884dce3eae9d3eb0b0d49600c1260b90.
💬 achow101 commented on pull request "wallet: fix deadlock in bdb read write operation":
(https://github.com/bitcoin/bitcoin/pull/27556#issuecomment-1553197160)
ACK 69d43905b7f1d4849dfaea1b5744ee473ccc8744
(https://github.com/bitcoin/bitcoin/pull/27556#issuecomment-1553197160)
ACK 69d43905b7f1d4849dfaea1b5744ee473ccc8744
💬 sdaftuar commented on pull request "test: Add test to check tx in the last block can be downloaded":
(https://github.com/bitcoin/bitcoin/pull/27695#issuecomment-1553197793)
ACK fa4c16b186a34f4172deda617166813c8cb92c59
(https://github.com/bitcoin/bitcoin/pull/27695#issuecomment-1553197793)
ACK fa4c16b186a34f4172deda617166813c8cb92c59
💬 kallerosenbaum commented on issue "bitcoin-qt fails to parse binary psbt file":
(https://github.com/bitcoin/bitcoin/issues/27697#issuecomment-1553202155)
Thanks for you quick responses. Will use 25rc2 instead then.
(https://github.com/bitcoin/bitcoin/issues/27697#issuecomment-1553202155)
Thanks for you quick responses. Will use 25rc2 instead then.
🚀 achow101 merged a pull request: "wallet: fix deadlock in bdb read write operation"
(https://github.com/bitcoin/bitcoin/pull/27556)
(https://github.com/bitcoin/bitcoin/pull/27556)
💬 sdaftuar commented on pull request "p2p: Stop relaying non-mempool txs":
(https://github.com/bitcoin/bitcoin/pull/27625#issuecomment-1553225738)
> I think it would also be good to continue serving txs from vExtraTxnForCompact (and to use them for resolving orphans?) to help address the https://github.com/bitcoin/bitcoin/pull/17303#issuecomment-550460346, but I don't think that needs to be a prerequisite for dropping mapRelay.
I don't think the privacy issues raised by not relaying replaced transactions are significant at all. There are many ways to use transaction relay behavior to infer the network topology, and I think there are do
...
(https://github.com/bitcoin/bitcoin/pull/27625#issuecomment-1553225738)
> I think it would also be good to continue serving txs from vExtraTxnForCompact (and to use them for resolving orphans?) to help address the https://github.com/bitcoin/bitcoin/pull/17303#issuecomment-550460346, but I don't think that needs to be a prerequisite for dropping mapRelay.
I don't think the privacy issues raised by not relaying replaced transactions are significant at all. There are many ways to use transaction relay behavior to infer the network topology, and I think there are do
...
💬 fanquake commented on pull request "net, refactor: net_processing, add `ProcessCompactBlockTxns`":
(https://github.com/bitcoin/bitcoin/pull/26969#issuecomment-1553226812)
@instagibbs this conflicts with #27626, thoughts?
(https://github.com/bitcoin/bitcoin/pull/26969#issuecomment-1553226812)
@instagibbs this conflicts with #27626, thoughts?
💬 brunoerg commented on pull request "test: fix intermittent issue in `feature_bip68_sequence`":
(https://github.com/bitcoin/bitcoin/pull/27177#issuecomment-1553231704)
Thanks everyone for reviewing! Here's what have changed:
- `get_utxo` will not return immature coinbase utxos anymore
- We won't use the `confirmation` count from utxo object since it may have outdated. We will use current block height + utxo's height for it
- changed `get_utxos` to use current block height to compute the confirmation count instead of using the value from utxo object, same logic from `get_utxo`.
(https://github.com/bitcoin/bitcoin/pull/27177#issuecomment-1553231704)
Thanks everyone for reviewing! Here's what have changed:
- `get_utxo` will not return immature coinbase utxos anymore
- We won't use the `confirmation` count from utxo object since it may have outdated. We will use current block height + utxo's height for it
- changed `get_utxos` to use current block height to compute the confirmation count instead of using the value from utxo object, same logic from `get_utxo`.
💬 sdaftuar commented on pull request "net_processing: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1197968591)
The `NodeClock::now()` time can go backwards, right? If so I think that could pose some issues if we're primarily relying on system time in order to determine whether transactions should be available for relay. Can we use a steady clock instead?
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1197968591)
The `NodeClock::now()` time can go backwards, right? If so I think that could pose some issues if we're primarily relying on system time in order to determine whether transactions should be available for relay. Can we use a steady clock instead?
💬 sdaftuar commented on pull request "net_processing: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#issuecomment-1553249207)
Concept ACK on getting rid of the recently announced filter. I think we'd need to use a steady clock to avoid random failures for no good reason, is that easy to do?
(https://github.com/bitcoin/bitcoin/pull/27675#issuecomment-1553249207)
Concept ACK on getting rid of the recently announced filter. I think we'd need to use a steady clock to avoid random failures for no good reason, is that easy to do?
💬 fanquake commented on issue "One core in CPU usage rate remains at 100% for a long time, causing serious delays in new blocks and forks":
(https://github.com/bitcoin/bitcoin/issues/27623#issuecomment-1553254260)
@huzhenyuan v24.1 is now available: https://github.com/bitcoin/bitcoin/releases/tag/v24.1.
(https://github.com/bitcoin/bitcoin/issues/27623#issuecomment-1553254260)
@huzhenyuan v24.1 is now available: https://github.com/bitcoin/bitcoin/releases/tag/v24.1.
👍 ryanofsky approved a pull request: "kernel: Remove interface_ui, util/system from kernel library"
(https://github.com/bitcoin/bitcoin/pull/27636#pullrequestreview-1432884414)
Code review ACK 861582b4087f1602e7115ee4a70ef7a7263eb36b. Just suggested changes since last review. (Thanks!)
(https://github.com/bitcoin/bitcoin/pull/27636#pullrequestreview-1432884414)
Code review ACK 861582b4087f1602e7115ee4a70ef7a7263eb36b. Just suggested changes since last review. (Thanks!)
💬 ryanofsky commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1197937840)
In commit "refactor: Split util::insert into its own file" (d492a2e5c88257cb14c417615dba525ce2cd9cfc)
Does this file need to `#include <set>`. Just wondering if IWYU would complain.
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1197937840)
In commit "refactor: Split util::insert into its own file" (d492a2e5c88257cb14c417615dba525ce2cd9cfc)
Does this file need to `#include <set>`. Just wondering if IWYU would complain.
💬 ryanofsky commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1197984670)
In commit "kernel: Add fatalError method to notifications" (861582b4087f1602e7115ee4a70ef7a7263eb36b)
I think it would make sense to get rid of this callback now that there is a notifications object. Following seems to work:
```diff
diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp
index 41056272f549..3a055c8e0bc7 100644
--- a/src/node/chainstate.cpp
+++ b/src/node/chainstate.cpp
@@ -200,10 +200,7 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const C
...
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1197984670)
In commit "kernel: Add fatalError method to notifications" (861582b4087f1602e7115ee4a70ef7a7263eb36b)
I think it would make sense to get rid of this callback now that there is a notifications object. Following seems to work:
```diff
diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp
index 41056272f549..3a055c8e0bc7 100644
--- a/src/node/chainstate.cpp
+++ b/src/node/chainstate.cpp
@@ -200,10 +200,7 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const C
...
💬 instagibbs commented on pull request "net, refactor: net_processing, add `ProcessCompactBlockTxns`":
(https://github.com/bitcoin/bitcoin/pull/26969#issuecomment-1553274722)
At first glance, this doesn't seem to interfere in any meaningful way with #27626
I promise to review this as a follow-up cleanup :pray:
(https://github.com/bitcoin/bitcoin/pull/26969#issuecomment-1553274722)
At first glance, this doesn't seem to interfere in any meaningful way with #27626
I promise to review this as a follow-up cleanup :pray:
💬 fanquake commented on pull request "net, refactor: net_processing, add `ProcessCompactBlockTxns`":
(https://github.com/bitcoin/bitcoin/pull/26969#issuecomment-1553277212)
> I promise to review this as a follow-up cleanup 🙏
Ok. This could be drafted for now / rebased on top of #27626?
(https://github.com/bitcoin/bitcoin/pull/26969#issuecomment-1553277212)
> I promise to review this as a follow-up cleanup 🙏
Ok. This could be drafted for now / rebased on top of #27626?