🚀 glozow merged a pull request: "test: refactor out same-txid-diff-wtxid tx to reuse in other tests"
(https://github.com/bitcoin/bitcoin/pull/32385)
(https://github.com/bitcoin/bitcoin/pull/32385)
💬 luke-jr commented on pull request "ci: Catch tests corrupting the source directory":
(https://github.com/bitcoin/bitcoin/pull/32874#issuecomment-3046462509)
Better not to trust CI to do a security audit for you...
One benefit of CMake is that you can build as another user without write access to the source dir (or indeed, in an isolated container).
(https://github.com/bitcoin/bitcoin/pull/32874#issuecomment-3046462509)
Better not to trust CI to do a security audit for you...
One benefit of CMake is that you can build as another user without write access to the source dir (or indeed, in an isolated container).
📝 ryanofsky opened a pull request: "POC: IPC tracing interface"
(https://github.com/bitcoin/bitcoin/pull/32898)
This a is proof-of-concept adding an IPC interface for tracing as discussed in https://github.com/bitcoin-core/libmultiprocess/issues/185. As mentioned there, this is a naive implementation that would need to be optimized to have good performance, but it does show an end-to-end demo of how tracing could work using IPC.
The can be tested by building with `-DENABLE_IPC=ON` and running:
```bash
build/bin/bitcoin-node -regtest -ipcbind=unix # Start node listening on IPC socket
build/bin/bit
...
(https://github.com/bitcoin/bitcoin/pull/32898)
This a is proof-of-concept adding an IPC interface for tracing as discussed in https://github.com/bitcoin-core/libmultiprocess/issues/185. As mentioned there, this is a naive implementation that would need to be optimized to have good performance, but it does show an end-to-end demo of how tracing could work using IPC.
The can be tested by building with `-DENABLE_IPC=ON` and running:
```bash
build/bin/bitcoin-node -regtest -ipcbind=unix # Start node listening on IPC socket
build/bin/bit
...
💬 fanquake commented on pull request "cmake: Use `AUTHOR_WARNING` for warnings":
(https://github.com/bitcoin/bitcoin/pull/32865#issuecomment-3046485662)
> Concept NACK: These warnings don't appear to be just for developers, but also relevant to builders.
Not sure I understand your NACK. The warnings will still be shown to builders; using `AUTHOR_WARNING` just gives us a proper mechanism to turn warnings into errors.
(https://github.com/bitcoin/bitcoin/pull/32865#issuecomment-3046485662)
> Concept NACK: These warnings don't appear to be just for developers, but also relevant to builders.
Not sure I understand your NACK. The warnings will still be shown to builders; using `AUTHOR_WARNING` just gives us a proper mechanism to turn warnings into errors.
📝 ryanofsky converted_to_draft a pull request: "POC: IPC tracing interface"
(https://github.com/bitcoin/bitcoin/pull/32898)
This a is proof-of-concept adding an IPC interface for tracing as discussed in https://github.com/bitcoin-core/libmultiprocess/issues/185. As mentioned there, this is a naive implementation that would need to be optimized to have good performance, but it does show an end-to-end demo of how tracing could work using IPC.
The can be tested by building with `-DENABLE_IPC=ON` and running:
```bash
build/bin/bitcoin-node -regtest -ipcbind=unix # Start node listening on IPC socket
build/bin/bit
...
(https://github.com/bitcoin/bitcoin/pull/32898)
This a is proof-of-concept adding an IPC interface for tracing as discussed in https://github.com/bitcoin-core/libmultiprocess/issues/185. As mentioned there, this is a naive implementation that would need to be optimized to have good performance, but it does show an end-to-end demo of how tracing could work using IPC.
The can be tested by building with `-DENABLE_IPC=ON` and running:
```bash
build/bin/bitcoin-node -regtest -ipcbind=unix # Start node listening on IPC socket
build/bin/bit
...
💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-3046495307)
Each transaction now counts as `1 + num_inputs / 10` announcements, accounting for the additional time it takes to do `m_outpoint_to_orphan_it` operations (benchmarks suggest it's roughly 1/10th of the time to do `m_orphans` operations). We round down so that the first 9 inputs are "free" and most normal transactions don't get a "penalty". This means a 9-input transaction has the highest computation time / latency score, which is why the benchmarks use them.
A few pushes ago I changed the lim
...
(https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-3046495307)
Each transaction now counts as `1 + num_inputs / 10` announcements, accounting for the additional time it takes to do `m_outpoint_to_orphan_it` operations (benchmarks suggest it's roughly 1/10th of the time to do `m_orphans` operations). We round down so that the first 9 inputs are "free" and most normal transactions don't get a "penalty". This means a 9-input transaction has the highest computation time / latency score, which is why the benchmarks use them.
A few pushes ago I changed the lim
...
💬 murchandamus commented on pull request "[WIP] wallet: tx creation, don't select outputs from txes that are being replaced":
(https://github.com/bitcoin/bitcoin/pull/26732#issuecomment-3046495601)
@furszy: #27286 was merged, in case this is still relevant.
(https://github.com/bitcoin/bitcoin/pull/26732#issuecomment-3046495601)
@furszy: #27286 was merged, in case this is still relevant.
💬 luke-jr commented on pull request "index: Fix missing case in the comment in NextSyncBlock()":
(https://github.com/bitcoin/bitcoin/pull/32875#issuecomment-3046501091)
```c++
CBlockIndex* NextInclRewind(const CBlockIndex* pindex) const
{
if (!Contains(Assert(pindex))) {
pindex = Assert(FindFork(pindex));
}
return (*this)[pindex->nHeight + 1];
}
```
(https://github.com/bitcoin/bitcoin/pull/32875#issuecomment-3046501091)
```c++
CBlockIndex* NextInclRewind(const CBlockIndex* pindex) const
{
if (!Contains(Assert(pindex))) {
pindex = Assert(FindFork(pindex));
}
return (*this)[pindex->nHeight + 1];
}
```
💬 luke-jr commented on pull request "index: fix wrong assert of current_tip == m_best_block_index":
(https://github.com/bitcoin/bitcoin/pull/32878#issuecomment-3046511372)
The test should probably be added to the PR after the commit fixing it
(https://github.com/bitcoin/bitcoin/pull/32878#issuecomment-3046511372)
The test should probably be added to the PR after the commit fixing it
💬 instagibbs commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3046513244)
@ismaelsadeeq I don't think 1p1c is compatible with this approach at all, unless the connection is intentionally held open long enough for the parent to be fetched from the sending node? Obviously the connection could be held open with additional logic but just pointing this out. A sender-initiated package relay system might be easier to extend (but way out of scope of this PR).
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3046513244)
@ismaelsadeeq I don't think 1p1c is compatible with this approach at all, unless the connection is intentionally held open long enough for the parent to be fetched from the sending node? Obviously the connection could be held open with additional logic but just pointing this out. A sender-initiated package relay system might be easier to extend (but way out of scope of this PR).
💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-3046516046)
> A few pushes ago I changed the limit back from 24,000 to 3,000 after changing the benchmarks to be more representative of actual worst case and realizing we actually definitely can't handle 24k
Can you recap what the deficiency was?
(https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-3046516046)
> A few pushes ago I changed the limit back from 24,000 to 3,000 after changing the benchmarks to be more representative of actual worst case and realizing we actually definitely can't handle 24k
Can you recap what the deficiency was?
💬 luke-jr commented on pull request "index: remove unnecessary locater cleaning in BaseIndex::Init()":
(https://github.com/bitcoin/bitcoin/pull/32882#discussion_r2191008997)
No harm in returning success, even if ignored currently.
(https://github.com/bitcoin/bitcoin/pull/32882#discussion_r2191008997)
No harm in returning success, even if ignored currently.
💬 hodlinator commented on pull request "refactor: Header sync optimisations & simplifications":
(https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2191012890)
meta: There's an issue with the `https://github.com/bitcoin/bitcoin/commit/09c7d0f703067d455aae4ee458ba7953c29d72fb#r2149379657` style links (taken from outdated "File Changes"-tab?). I've switched to mostly grabbing links from the "Conversation"-tab `https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2149379657`.
(https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2191012890)
meta: There's an issue with the `https://github.com/bitcoin/bitcoin/commit/09c7d0f703067d455aae4ee458ba7953c29d72fb#r2149379657` style links (taken from outdated "File Changes"-tab?). I've switched to mostly grabbing links from the "Conversation"-tab `https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2149379657`.
💬 furszy commented on pull request "[WIP] wallet: tx creation, don't select outputs from txes that are being replaced":
(https://github.com/bitcoin/bitcoin/pull/26732#issuecomment-3046561786)
> @furszy: #27286 was merged, in case this is still relevant.
Thx. It is still relevant but I don't have enough time to tackle it. Up for grabs.
(https://github.com/bitcoin/bitcoin/pull/26732#issuecomment-3046561786)
> @furszy: #27286 was merged, in case this is still relevant.
Thx. It is still relevant but I don't have enough time to tackle it. Up for grabs.
👋 sipa's pull request is ready for review: "cluster mempool: add TxGraph work controls"
(https://github.com/bitcoin/bitcoin/pull/32263)
(https://github.com/bitcoin/bitcoin/pull/32263)
💬 sipa commented on pull request "cluster mempool: add TxGraph work controls":
(https://github.com/bitcoin/bitcoin/pull/32263#issuecomment-3046566193)
Rebased after the merge of #31553, and undrafted.
(https://github.com/bitcoin/bitcoin/pull/32263#issuecomment-3046566193)
Rebased after the merge of #31553, and undrafted.
💬 brunoerg commented on issue "test: break `feature_block` into subtests?":
(https://github.com/bitcoin/bitcoin/issues/32877#issuecomment-3046576815)
> Are you sure? I'd say the reorg test is useful to perform on a "dirty" state.
I agree that in this case is useful, but not sure about the other tests. But anyway, since the reorg is the slowest test, running it without reorg should be fine. So we could have just an option to skip it?
(https://github.com/bitcoin/bitcoin/issues/32877#issuecomment-3046576815)
> Are you sure? I'd say the reorg test is useful to perform on a "dirty" state.
I agree that in this case is useful, but not sure about the other tests. But anyway, since the reorg is the slowest test, running it without reorg should be fine. So we could have just an option to skip it?
💬 achow101 commented on pull request "wallet: Remove watchonly behavior and isminetypes":
(https://github.com/bitcoin/bitcoin/pull/32523#issuecomment-3046770880)
#32618 merged, ready for review now.
(https://github.com/bitcoin/bitcoin/pull/32523#issuecomment-3046770880)
#32618 merged, ready for review now.
👋 achow101's pull request is ready for review: "wallet: Remove watchonly behavior and isminetypes"
(https://github.com/bitcoin/bitcoin/pull/32523)
(https://github.com/bitcoin/bitcoin/pull/32523)
💬 achow101 commented on pull request "wallet, rpc: Move (Un)LockCoin WalletBatch creation out of RPC":
(https://github.com/bitcoin/bitcoin/pull/32593#discussion_r2191169711)
If I need to retouch.
(https://github.com/bitcoin/bitcoin/pull/32593#discussion_r2191169711)
If I need to retouch.