🚀 fanquake merged a pull request: "ci: compile Clang and compiler-rt in msan jobs"
(https://github.com/bitcoin/bitcoin/pull/27737)
(https://github.com/bitcoin/bitcoin/pull/27737)
🤔 glozow reviewed a pull request: "Fuzz: Mitigate timeout in CalculateTotalBumpFees"
(https://github.com/bitcoin/bitcoin/pull/27803#pullrequestreview-1457035240)
ACK 5d718f6913219d3ebe8394a17ddee81915e6f0ac
The code changes make sense to me. Reproduced the slow input from #27799, and it is dramatically faster with this change.
(https://github.com/bitcoin/bitcoin/pull/27803#pullrequestreview-1457035240)
ACK 5d718f6913219d3ebe8394a17ddee81915e6f0ac
The code changes make sense to me. Reproduced the slow input from #27799, and it is dramatically faster with this change.
✅ glozow closed an issue: "fuzz: mini_miner: Timeout in mini_miner"
(https://github.com/bitcoin/bitcoin/issues/27799)
(https://github.com/bitcoin/bitcoin/issues/27799)
🚀 glozow merged a pull request: "Fuzz: Mitigate timeout in CalculateTotalBumpFees"
(https://github.com/bitcoin/bitcoin/pull/27803)
(https://github.com/bitcoin/bitcoin/pull/27803)
💬 willcl-ark commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1214180938)
Can you explain the rationale behind the 60 hours chosen? Also perhaps you can update OP as it still mentions 12 hours :)
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1214180938)
Can you explain the rationale behind the 60 hours chosen? Also perhaps you can update OP as it still mentions 12 hours :)
💬 theStack commented on pull request "rpc: remove deprecated "warning" field from {create,load,restore,unload}wallet":
(https://github.com/bitcoin/bitcoin/pull/27757#discussion_r1214199866)
Thanks, included the move back of the `UnloadWallet` call and added you as co-author.
(https://github.com/bitcoin/bitcoin/pull/27757#discussion_r1214199866)
Thanks, included the move back of the `UnloadWallet` call and added you as co-author.
💬 theStack commented on pull request "rpc: remove deprecated "warning" field from {create,load,restore,unload}wallet":
(https://github.com/bitcoin/bitcoin/pull/27757#discussion_r1214200031)
Thanks, done.
(https://github.com/bitcoin/bitcoin/pull/27757#discussion_r1214200031)
Thanks, done.
💬 theStack commented on pull request "rpc: remove deprecated "warning" field from {create,load,restore,unload}wallet":
(https://github.com/bitcoin/bitcoin/pull/27757#issuecomment-1573510436)
@jonatack: Oh sorry, wasn't aware you already put time into this. Pulled in your documentation fixup commit and took also your other suggestions for the removal and release note commits.
(https://github.com/bitcoin/bitcoin/pull/27757#issuecomment-1573510436)
@jonatack: Oh sorry, wasn't aware you already put time into this. Pulled in your documentation fixup commit and took also your other suggestions for the removal and release note commits.
💬 glozow commented on pull request "[NO MERGE] BIP331 Ancestor Package Relay":
(https://github.com/bitcoin/bitcoin/pull/27742#issuecomment-1573521731)
> > redownloading orphans if we cannot afford to protect them
>
> Is the idea here that protection is a bandiwdth optimization to avoid fetching an orphan twice in a row in the presence of malicious peers or random churn? Reasonable if so, just want this to be explicitly stated if so.
Yes exactly. Worse is we want to avoid a situation where we requested the rest of the package while the orphan was in our orphanage, but then it fell out while we were waiting for the rest of the transactions
...
(https://github.com/bitcoin/bitcoin/pull/27742#issuecomment-1573521731)
> > redownloading orphans if we cannot afford to protect them
>
> Is the idea here that protection is a bandiwdth optimization to avoid fetching an orphan twice in a row in the presence of malicious peers or random churn? Reasonable if so, just want this to be explicitly stated if so.
Yes exactly. Worse is we want to avoid a situation where we requested the rest of the package while the orphan was in our orphanage, but then it fell out while we were waiting for the rest of the transactions
...
💬 TheCharlatan commented on pull request "kernel: Remove shutdown from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27711#discussion_r1214254203)
I don't really understand the improvement in the context of achieving step 2 of the kernel library project between the solution you implemented now and what is implemented currently here in the first commit, besides it being a big step towards de-globalizing. One of my goals for this PR here is to stop the kernel relying on code that handles platform-dependent blocking while still being signal-aware (e.g. the bulk of what is currently in shutdown). I don't see why such logic should exist in a st
...
(https://github.com/bitcoin/bitcoin/pull/27711#discussion_r1214254203)
I don't really understand the improvement in the context of achieving step 2 of the kernel library project between the solution you implemented now and what is implemented currently here in the first commit, besides it being a big step towards de-globalizing. One of my goals for this PR here is to stop the kernel relying on code that handles platform-dependent blocking while still being signal-aware (e.g. the bulk of what is currently in shutdown). I don't see why such logic should exist in a st
...
💬 ChrisCho-H commented on pull request "script: add description for the functionality of each opcode":
(https://github.com/bitcoin/bitcoin/pull/27109#issuecomment-1573587610)
@MarcoFalke would u mind to review this?
(https://github.com/bitcoin/bitcoin/pull/27109#issuecomment-1573587610)
@MarcoFalke would u mind to review this?
💬 ryanofsky commented on pull request "walletdb: Add PrefixCursor":
(https://github.com/bitcoin/bitcoin/pull/27790#discussion_r1214272570)
I think I agree that current `Serialize(Span<char>)` overloads could be a little dangerous:
https://github.com/bitcoin/bitcoin/blob/6a560aceb75e618f3106a8850e053cd8de87616a/src/serialize.h#L202-L205
Because if a variable-length container could be implicitly converted to span of these types, it might incorrectly match one of these overloads and be serialized as a fixed length span without a length prefix, and no longer be deserializable as the original container.
If that is a real proble
...
(https://github.com/bitcoin/bitcoin/pull/27790#discussion_r1214272570)
I think I agree that current `Serialize(Span<char>)` overloads could be a little dangerous:
https://github.com/bitcoin/bitcoin/blob/6a560aceb75e618f3106a8850e053cd8de87616a/src/serialize.h#L202-L205
Because if a variable-length container could be implicitly converted to span of these types, it might incorrectly match one of these overloads and be serialized as a fixed length span without a length prefix, and no longer be deserializable as the original container.
If that is a real proble
...
🤔 Sjors reviewed a pull request: "contrib: verify torrents with verify-binary.py"
(https://github.com/bitcoin/bitcoin/pull/27762#pullrequestreview-1457289222)
Successfully tested on macOS 13.4 against v25.0.
Lightly code reviewed a6dcf2cb0817143beacf0b9004191a0dd6d2e43c.
Do we want to use the .asc file from the torrent, or should we just get it from the Guix signature repo? The latter may have more signatures, as they keep coming in after release.
(https://github.com/bitcoin/bitcoin/pull/27762#pullrequestreview-1457289222)
Successfully tested on macOS 13.4 against v25.0.
Lightly code reviewed a6dcf2cb0817143beacf0b9004191a0dd6d2e43c.
Do we want to use the .asc file from the torrent, or should we just get it from the Guix signature repo? The latter may have more signatures, as they keep coming in after release.
💬 Sjors commented on pull request "contrib: verify torrents with verify-binary.py":
(https://github.com/bitcoin/bitcoin/pull/27762#discussion_r1214312611)
a6dcf2cb0817143beacf0b9004191a0dd6d2e43c Did you mean to change the indentation here? Maybe do so in the first refactor commit?
(https://github.com/bitcoin/bitcoin/pull/27762#discussion_r1214312611)
a6dcf2cb0817143beacf0b9004191a0dd6d2e43c Did you mean to change the indentation here? Maybe do so in the first refactor commit?
💬 Sjors commented on pull request "contrib: verify torrents with verify-binary.py":
(https://github.com/bitcoin/bitcoin/pull/27762#discussion_r1214314041)
a6dcf2cb0817143beacf0b9004191a0dd6d2e43c: did you mean to do this in the refactor commit?
(https://github.com/bitcoin/bitcoin/pull/27762#discussion_r1214314041)
a6dcf2cb0817143beacf0b9004191a0dd6d2e43c: did you mean to do this in the refactor commit?
💬 ryanofsky commented on pull request "kernel: Remove shutdown from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27711#discussion_r1214342650)
> One of my goals for this PR here is to stop the kernel relying on code that handles platform-dependent blocking while still being signal-aware (e.g. the bulk of what is currently in shutdown). I don't see why such logic should exist in a standalone kernel library. Do you have a different perspective here?
Yes, I don't think we should do this because it gets in the way of the kernel having a nice and future-proof API.
I think we both agree that applications using libbitcoinkernel should h
...
(https://github.com/bitcoin/bitcoin/pull/27711#discussion_r1214342650)
> One of my goals for this PR here is to stop the kernel relying on code that handles platform-dependent blocking while still being signal-aware (e.g. the bulk of what is currently in shutdown). I don't see why such logic should exist in a standalone kernel library. Do you have a different perspective here?
Yes, I don't think we should do this because it gets in the way of the kernel having a nice and future-proof API.
I think we both agree that applications using libbitcoinkernel should h
...
📝 willcl-ark opened a pull request: "init: deduplicate added connections"
(https://github.com/bitcoin/bitcoin/pull/27804)
Fixes #5299
Previously connections may be added to both m_added_nodes (-addnode) and m_specified_outgoing (-connect) if duplicated in config.
Deduplicate the lists before starting connection manager.
There are ways to bypass this simple de-duplication as it relies on string comparison, but it gets us most of the way.
(https://github.com/bitcoin/bitcoin/pull/27804)
Fixes #5299
Previously connections may be added to both m_added_nodes (-addnode) and m_specified_outgoing (-connect) if duplicated in config.
Deduplicate the lists before starting connection manager.
There are ways to bypass this simple de-duplication as it relies on string comparison, but it gets us most of the way.
💬 willcl-ark commented on pull request "init: deduplicate added connections":
(https://github.com/bitcoin/bitcoin/pull/27804#issuecomment-1573700827)
<details>
<summary>Before and after connection summaries</summary>
```sh
# Node 1
/home/will/src/bitcoin/src/bitcoin-cli -datadir=/tmp/node1-datadir getpeerinfo | jq '[.[] | {id: .id, addr: .addr, type: .connection_type}]'
[
{
"id": 0,
"addr": "127.0.0.1:60216",
"type": "inbound"
},
{
"id": 1,
"addr": "127.0.0.1:60218",
"type": "inbound"
},
{
"id": 2,
"addr": "127.0.0.1:18400",
"type": "manual"
}
]
# Node 2
/home/will/s
...
(https://github.com/bitcoin/bitcoin/pull/27804#issuecomment-1573700827)
<details>
<summary>Before and after connection summaries</summary>
```sh
# Node 1
/home/will/src/bitcoin/src/bitcoin-cli -datadir=/tmp/node1-datadir getpeerinfo | jq '[.[] | {id: .id, addr: .addr, type: .connection_type}]'
[
{
"id": 0,
"addr": "127.0.0.1:60216",
"type": "inbound"
},
{
"id": 1,
"addr": "127.0.0.1:60218",
"type": "inbound"
},
{
"id": 2,
"addr": "127.0.0.1:18400",
"type": "manual"
}
]
# Node 2
/home/will/s
...
💬 ismaelsadeeq commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1214368208)
Estimates that are 2.5 days old have nearly half the decay weight of the most recent block
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1214368208)
Estimates that are 2.5 days old have nearly half the decay weight of the most recent block
🚀 glozow merged a pull request: "test: added coverage to mining_basic.py"
(https://github.com/bitcoin/bitcoin/pull/27603)
(https://github.com/bitcoin/bitcoin/pull/27603)